-
Notifications
You must be signed in to change notification settings - Fork 19
Add option to save difference curve #237
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| **Added:** | ||
|
|
||
| * There is now an option to save the difference curve. This is computed on the common interval between the two curves. | ||
|
|
||
| **Changed:** | ||
|
|
||
| * <news item> | ||
|
|
||
| **Deprecated:** | ||
|
|
||
| * <news item> | ||
|
|
||
| **Removed:** | ||
|
|
||
| * <news item> | ||
|
|
||
| **Fixed:** | ||
|
|
||
| * <news item> | ||
|
|
||
| **Security:** | ||
|
|
||
| * <news item> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,14 +81,27 @@ def custom_error(self, msg): | |
| metavar="NAME", | ||
| dest="slocation", | ||
| help=( | ||
| "Save the manipulated PDF to a file named NAME. " | ||
| "Save the manipulated function to a file named NAME. " | ||
| "Use '-' for stdout.\n" | ||
| "When --multiple-<targets/morphs> is enabled, " | ||
| "save each manipulated PDF as a file in a directory named NAME;\n" | ||
| "you can specify names for each saved PDF file using " | ||
| "save each manipulated function as a file in a directory " | ||
| "named NAME;\n" | ||
| "you can specify names for each saved function file using " | ||
| "--save-names-file." | ||
| ), | ||
| ) | ||
| parser.add_option( | ||
| "--diff", | ||
| "--get-diff", | ||
| dest="get_diff", | ||
| action="store_true", | ||
| help=( | ||
| "Save the difference curve rather than the manipulated function.\n" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we do update the logic, update this help here too. |
||
| "This is computed as manipulated function minus target function.\n" | ||
| "The difference curve is computed on the interval shared by the " | ||
| "grid of the objective and target function." | ||
| ), | ||
| ) | ||
| parser.add_option( | ||
| "-v", | ||
| "--verbose", | ||
|
|
@@ -99,12 +112,12 @@ def custom_error(self, msg): | |
| parser.add_option( | ||
| "--rmin", | ||
| type="float", | ||
| help="Minimum r-value to use for PDF comparisons.", | ||
| help="Minimum r-value (abscissa) to use for function comparisons.", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "r" might be a bit confusing in our more general world of functions. Not sure what to change it to (and let's just make an issue, not do it on this PR0. "x" is widely used by scientists. "minimum value on the absisca" might be mathematically more correct? Anyway, just a note to make an issue.....
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, I noted this in #236 |
||
| ) | ||
| parser.add_option( | ||
| "--rmax", | ||
| type="float", | ||
| help="Maximum r-value to use for PDF comparisons.", | ||
| help="Maximum r-value (abscissa) to use for function comparisons.", | ||
| ) | ||
| parser.add_option( | ||
| "--tolerance", | ||
|
|
@@ -419,9 +432,9 @@ def custom_error(self, msg): | |
| "using a serial file NAMESFILE. The format of NAMESFILE should be " | ||
| "as follows: each target PDF is an entry in NAMESFILE. For each " | ||
| "entry, there should be a key {__save_morph_as__} whose value " | ||
| "specifies the name to save the manipulated PDF as. An example " | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. another issue to make as claning before release (not now) would be to do a global search for PDF and root it out wherever it survives. Like a weed......
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, noted in #236, didn't want to do that now |
||
| ".json serial file is included in the tutorial directory " | ||
| "on the package GitHub repository." | ||
| "specifies the name to save the manipulated function as." | ||
| "An example .json serial file is included in the tutorial " | ||
| "directory on the package GitHub repository." | ||
| ), | ||
| ) | ||
| group.add_option( | ||
|
|
@@ -492,10 +505,7 @@ def single_morph( | |
| smear_in = "None" | ||
| hshift_in = "None" | ||
| vshift_in = "None" | ||
| config = {} | ||
| config["rmin"] = opts.rmin | ||
| config["rmax"] = opts.rmax | ||
| config["rstep"] = None | ||
| config = {"rmin": opts.rmin, "rmax": opts.rmax, "rstep": None} | ||
| if ( | ||
| opts.rmin is not None | ||
| and opts.rmax is not None | ||
|
|
@@ -708,13 +718,29 @@ def single_morph( | |
| morph_results.update({"Pearson": pcc}) | ||
|
|
||
| # Print summary to terminal and save morph to file if requested | ||
| xy_save = [chain.x_morph_out, chain.y_morph_out] | ||
| if opts.get_diff is not None: | ||
| diff_chain = morphs.MorphChain( | ||
| {"rmin": None, "rmax": None, "rstep": None} | ||
| ) | ||
| diff_chain.append(morphs.MorphRGrid()) | ||
| diff_chain( | ||
| chain.x_morph_out, | ||
| chain.y_morph_out, | ||
| chain.x_target_in, | ||
| chain.y_target_in, | ||
| ) | ||
| xy_save = [ | ||
| diff_chain.x_morph_out, | ||
| diff_chain.y_morph_out - diff_chain.y_target_out, | ||
| ] | ||
| try: | ||
| io.single_morph_output( | ||
| morph_inputs, | ||
| morph_results, | ||
| save_file=opts.slocation, | ||
| morph_file=pargs[0], | ||
| xy_out=[chain.x_morph_out, chain.y_morph_out], | ||
| xy_out=xy_save, | ||
| verbose=opts.verbose, | ||
| stdout_flag=stdout_flag, | ||
| ) | ||
|
|
@@ -753,7 +779,7 @@ def single_morph( | |
| # Return different things depending on whether it is python interfaced | ||
| if python_wrap: | ||
| morph_info = morph_results | ||
| morph_table = numpy.array([chain.x_morph_out, chain.y_morph_out]).T | ||
| morph_table = numpy.array(xy_save).T | ||
| return morph_info, morph_table | ||
| else: | ||
| return morph_results | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we want to do either/or here. A clearer logic would be that diff is saved if this is true, not saved if it is false, and also the case for the morph function. So TT would save both. To keep the API cleaner, we could have defaults of TF for morph and diff (the current behavior).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, created #238 since I think we should have a discussion on this.