11 poc minimizer factory #13
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| from .minimizer_bumps import Bumps | ||
| from .minimizer_dfo import DFO | ||
| from .minimizer_lmfit import LMFit |
There was a problem hiding this comment.
Any of these will throw if the given minimizer is not installed.
Originally (__init__.py), we would allow failures but kept going after notifying the user.
Why this (inflexible) choice?
There was a problem hiding this comment.
The code should now be able to run with or without all the minimizer engines.
Furthermore, the factory now explicitly list the minimizers methods from the minimizer engines that are directly supported
| self._update_minimizer(DEFAULT_MINIMIZER) | ||
|
|
||
| def __initialize(self): | ||
| def create(self, minimizer_name: str = DEFAULT_MINIMIZER) -> None: |
There was a problem hiding this comment.
What is the purpose of the create method? Seems to be redundant now.
There was a problem hiding this comment.
Yes, you are right, but I did not dare to remove it yet since it is a public method and might be used in other codes.
I will add a code comment that I should be cleaned up.
| def convert_to_pars_obj(self, pars) -> object: | ||
| return self._minimizer.convert_to_pars_obj(pars) | ||
|
|
||
| def initialize(self, fit_object, fit_function: Callable) -> None: |
There was a problem hiding this comment.
Seems to me like the initialize method is simply a combined setter method for the fit_object and the fit_function. So isn't this method also redundant?
There was a problem hiding this comment.
You are right again. I will add a code comment and we can clean up these seemingly redundant methods at a later state.
I have also added an issue: #15
3 participants
This PR is a change to how we in
Fitteraccess different minimizers as described in ADR suggestionThe largest changes are in the
fitter.pyFitternow relies on a factory to produce the required minimizer.All minimizers are a child of
MinimizerBase.The factory will:
MinimizerBaseclass.Fittershould therefore only use the minimizer methods defined inMinimizerBase