-
Notifications
You must be signed in to change notification settings - Fork 23
feat: Initialize FitRecipe with a results file or object #166
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
base: v3.3.0
Are you sure you want to change the base?
Changes from 8 commits
698842e
12ee91c
0d6e1be
474b32e
96ac076
66a658d
83fd535
0c25030
0625461
17f2b10
9e205d7
39178a6
73704f4
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:** | ||
|
|
||
| * Added ``initialize_recipe_with_results`` to ``FitRecipe``. | ||
|
|
||
| **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 |
|---|---|---|
|
|
@@ -35,11 +35,13 @@ | |
| __all__ = ["FitRecipe"] | ||
|
|
||
| from collections import OrderedDict | ||
| from pathlib import Path | ||
|
|
||
| import matplotlib.pyplot as plt | ||
| from bg_mpl_stylesheets.styles import all_styles | ||
| from numpy import array, concatenate, dot, sqrt | ||
|
|
||
| import diffpy.srfit.util.inpututils as utils | ||
| from diffpy.srfit.fitbase.fithook import PrintFitHook | ||
| from diffpy.srfit.fitbase.parameter import ParameterProxy | ||
| from diffpy.srfit.fitbase.recipeorganizer import RecipeOrganizer | ||
|
|
@@ -1184,6 +1186,74 @@ def initialize_recipe_with_recipe(self, recipe_object): | |
| if restraint not in self._restraints: | ||
| self._restraints.add(restraint) | ||
|
|
||
| def _pretty_print_results_dict(self, params_dict): | ||
| """Pretty print a dictionary of parameter names and values.""" | ||
| sorted_params = sorted(params_dict.items()) | ||
| width = max(len(name) for name, _ in sorted_params) | ||
| for name, value in sorted_params: | ||
| if isinstance(value, float): | ||
| value_str = f"{value:.6g}" | ||
| else: | ||
| value_str = str(value) | ||
| print(f" {name:<{width}} = {value_str}") | ||
|
|
||
| def _set_parameters_from_dict(self, params_dict): | ||
| """Set the parameters of the FitRecipe from a dictionary of | ||
| parameter names and values.""" | ||
| for param_name, param_value in params_dict.items(): | ||
| if param_name in self._parameters: | ||
| self._parameters[param_name].setValue(param_value) | ||
| else: | ||
| print( | ||
| f"Warning: Parameter '{param_name}' from results " | ||
| "not found in FitRecipe and will be ignored." | ||
| ) | ||
|
|
||
| def initialize_recipe_with_results(self, results, verbose=True): | ||
| """Initialize a FitRecipe with a FitResults object or a results | ||
| file. | ||
|
|
||
| Note that at least one FitContribution must already exist in | ||
| the FitRecipe. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| results : FitResults, pathlib.Path, or str | ||
| The FitResults object or path to results file to initialize with. | ||
| verbose : bool, optional | ||
| If True, print warnings for any parameters in the results that are | ||
| not in the FitRecipe. Default is True. | ||
|
|
||
| Raises | ||
| ------ | ||
| ValueError | ||
| If the input results is not a FitResults object or a path to a | ||
| results file. | ||
| """ | ||
| if hasattr(results, "print_results"): | ||
| params_dict = utils.get_dict_from_results_object(results) | ||
| elif isinstance(results, (str, Path)): | ||
| params_dict = utils.get_dict_from_results_file(results) | ||
| else: | ||
| raise ValueError( | ||
| "The input results must be a FitResults object or a path to a " | ||
| f"results file, but got {type(results)}." | ||
| ) | ||
| self._set_parameters_from_dict(params_dict) | ||
| if verbose: | ||
| print() | ||
| print("Parameters found in Results:") | ||
| print("=" * 30) | ||
| self._pretty_print_results_dict(params_dict) | ||
| print() | ||
| print("Parameters set in FitRecipe:") | ||
| print("=" * 30) | ||
| set_parameters_dict = { | ||
| param.name: param.getValue() | ||
| for param in self._parameters.values() | ||
| } | ||
| self._pretty_print_results_dict(set_parameters_dict) | ||
|
Contributor
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. Prints the parameters found in the results and the parameters set |
||
|
|
||
| def set_plot_defaults(self, **kwargs): | ||
| """Set default plotting options for all future plots. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -150,19 +150,23 @@ def _capturestdout(f, *args, **kwargs): | |
| @pytest.fixture() | ||
| def build_recipe_one_contribution(): | ||
| "helper to build a simple recipe" | ||
| profile = Profile() | ||
| x = linspace(0, pi, 10) | ||
| y = sin(x) | ||
| profile.set_observed_profile(x, y) | ||
| contribution = FitContribution("c1") | ||
| contribution.set_profile(profile) | ||
| contribution.set_equation("amplitude*sin(wave_number*x + phase_shift)") | ||
| recipe = FitRecipe() | ||
| recipe.add_contribution(contribution) | ||
| recipe.add_variable(contribution.amplitude, 1) | ||
| recipe.add_variable(contribution.wave_number, 1) | ||
| recipe.add_variable(contribution.phase_shift, 1) | ||
| return recipe | ||
|
|
||
| def _build_recipe(): | ||
| profile = Profile() | ||
| x = linspace(0, pi, 10) | ||
|
||
| y = sin(x) | ||
| profile.set_observed_profile(x, y) | ||
| contribution = FitContribution("c1") | ||
| contribution.set_profile(profile) | ||
| contribution.set_equation("amplitude*sin(wave_number*x + phase_shift)") | ||
| recipe = FitRecipe() | ||
| recipe.add_contribution(contribution) | ||
| recipe.add_variable(contribution.amplitude, 4) | ||
| recipe.add_variable(contribution.wave_number, 3) | ||
| recipe.add_variable(contribution.phase_shift, 2) | ||
| return recipe | ||
|
|
||
|
Contributor
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. had to wrap this in a second function because I realized calling the same fixture twice after the first one was refined led to initial values in the second call being the values of the previously refined recipe
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. I think you can simply change the scope of the fixture to remove this behavior
Contributor
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. @sbillinge Unfortunately that doesn't work here. It was already set to
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. ok, but I am confused. in this case it would reset every time through a
Contributor
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. @sbillinge In the current version, the test fixture is not being called twice when you do, What is happening here is that it is assigning the same fixture value to two variable names so When we wrap it in another function like in the incoming version, each recipe object can be created
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. This may be indicating a weakness with our code, either the test or the code itself. Is it fixed if you instantiate recipe1 and recipe2 both at the top of the testing function?
Contributor
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. @sbillinge I tested this without the conftest fixture and built the recipes manually like so, Doing this passes the test meaning its a fixture related thing and not a code related thing, what we could do is have it like this for this specific test (and i think one other) and revert the conftest fixture back to original.
Contributor
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.
@sbillinge and no, this doesnt fix it |
||
| return _build_recipe | ||
|
|
||
|
|
||
| @pytest.fixture() | ||
|
|
||
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.
handles either a results object or a path to a results file