Conversation
|
Required for easyscience/EasyReflectometryApp#290 |
rozyczko
added
[scope] enhancement
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
damskii9992
left a comment
damskii9992
left a comment
There was a problem hiding this comment.
This PR needs an ADR explaining the software architecture design implemented and the rationale for why this design was chosen etc.
We probably also need a tutorial showcasing how the callback might be used. Both for ourselves as a kind of documentation but also for potential users.
I have not bothered looking at the unit tests.
| results.y_obs = self._cached_model.y | ||
| results.y_calc = self.evaluate(results.x, minimizer_parameters=results.p) | ||
| results.y_err = weights | ||
| results.n_evaluations = int(fit_results.nf) |
There was a problem hiding this comment.
shouldn't this be fit_results.nfev?
There was a problem hiding this comment.
No, this is correct for DFO-LS.
nf = number of objective evaluations
DFO doesn't follow SciPy style OptimizeResults API
| chi2_val = self.chi2 | ||
| reduced_val = self.reduced_chi2 | ||
| if not np.isfinite(chi2_val) or not np.isfinite(reduced_val): | ||
| raise ValueError |
There was a problem hiding this comment.
This error should probably have some context
There was a problem hiding this comment.
This ValueError is not a user-facing error. It is raised only to be caught immediately by the except Exception in line 54, after which chi2 and reduced_chi2 are rendered as N/A
There was a problem hiding this comment.
Even then, in the future we might change some code elsewhere to the catch exception and simply re-raise the error here. Without the error context, such re-raises would be hard to diagnose. It never hurts to have an descriptive error-message, even if it isn't currently being used.
There was a problem hiding this comment.
I don't think this is necessary, but I will begrudingly accept the request.
2 participants
This pull request adds support for progress callbacks during fitting operations. The main goal is to allow users to receive progress updates and optionally cancel fits in progress. Added
progress_callbackparameter and a way to handle cancellation gracefully.Added an optional
progress_callbackparameter to the fit methods inFitter,MinimizerBase,MultiFitterand all minimizer subclasses, allowing users to receive iterative progress updates and cancel fits.Implemented progress callback integration in
LMFit: added_create_iter_callbackto wrap the user callback, constructed detailed progress payloads, and enabled fit cancellation by raising a newFitCancelledexception. Ensured parameter values are restored on cancellation or error.Added the
FitCancelledexception class to signal user-requested cancellation, and ensured proper restoration of parameter values on cancellation or error.