Fitting routines exploration#36
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the documentation to demonstrate how Minuit2.jl integrates with Optimization.jl when using ComponentArrays. It introduces a new fit_minuit_nll helper function and refactors the tutorial to use the Optimization.jl solver interface. Review feedback recommends using the idiomatic loglikelihood function to improve performance, renaming variables to align with Optimization.jl conventions, and re-including parameter names in the low-level Minuit constructor for better debugging output.
| function nll(c, data, pars) | ||
| model = build_model(c, pars) | ||
| return -sum(logpdf.(Ref(model), data)) | ||
| end |
There was a problem hiding this comment.
Use loglikelihood from Distributions.jl instead of manually summing logpdf calls. This is more idiomatic and avoids the allocation of a temporary array during broadcasting, which can improve performance during optimization.
| function nll(c, data, pars) | |
| model = build_model(c, pars) | |
| return -sum(logpdf.(Ref(model), data)) | |
| end | |
| function nll(c, data, pars) | |
| model = build_model(c, pars) | |
| return -loglikelihood(model, data) | |
| end |
| function fit_minuit_nll( | ||
| constructor, | ||
| pars, | ||
| data; | ||
| minuit_settings = (strategy = 2, tolerance = 0.01, errordef = 0.5), | ||
| optimizer_settings = (maxiters = 100,), | ||
| problem_settings = (;), | ||
| ) | ||
| objective(pars) = nll(constructor, data, pars) | ||
| opf = OptimizationFunction((p, x) -> objective(p)) | ||
| opp = OptimizationProblem(opf, pars; problem_settings...) | ||
| return solve(opp, MigradOptimizer(; minuit_settings...); optimizer_settings...) | ||
| end |
There was a problem hiding this comment.
The variable name pars is used for both the initial guess argument and the optimization variable in the closure, which can be confusing. Using u0 for the initial guess and u for the optimization variable is more consistent with Optimization.jl conventions. Additionally, the objective closure can be simplified by calling nll directly in the OptimizationFunction.
| function fit_minuit_nll( | |
| constructor, | |
| pars, | |
| data; | |
| minuit_settings = (strategy = 2, tolerance = 0.01, errordef = 0.5), | |
| optimizer_settings = (maxiters = 100,), | |
| problem_settings = (;), | |
| ) | |
| objective(pars) = nll(constructor, data, pars) | |
| opf = OptimizationFunction((p, x) -> objective(p)) | |
| opp = OptimizationProblem(opf, pars; problem_settings...) | |
| return solve(opp, MigradOptimizer(; minuit_settings...); optimizer_settings...) | |
| end | |
| function fit_minuit_nll( | |
| constructor, | |
| u0, | |
| data; | |
| minuit_settings = (strategy = 2, tolerance = 0.01, errordef = 0.5), | |
| optimizer_settings = (maxiters = 100,), | |
| problem_settings = (;), | |
| ) | |
| opf = OptimizationFunction((u, p) -> nll(constructor, data, u)) | |
| opp = OptimizationProblem(opf, u0; problem_settings...) | |
| return solve(opp, MigradOptimizer(; minuit_settings...); optimizer_settings...) | |
| end |
| minuit = Minuit( | ||
| objective, | ||
| pars -> nll(constructor, data, pars), | ||
| start; | ||
| names = string.(keys(start)), | ||
| limits = limits, | ||
| error = collect(errors), | ||
| limits = collect(zip(lower, upper)), | ||
| error = collect(step), | ||
| arraycall = true, | ||
| errordef = 0.5, | ||
| strategy = 2, | ||
| tolerance = 0.01, | ||
| ) |
There was a problem hiding this comment.
Including the names argument in the Minuit constructor allows the optimizer to display meaningful parameter names in its output (e.g., during migrad! or when printing the minuit object). This significantly improves the debugging experience and maintains the functionality present in the previous version of the tutorial.
| minuit = Minuit( | |
| objective, | |
| pars -> nll(constructor, data, pars), | |
| start; | |
| names = string.(keys(start)), | |
| limits = limits, | |
| error = collect(errors), | |
| limits = collect(zip(lower, upper)), | |
| error = collect(step), | |
| arraycall = true, | |
| errordef = 0.5, | |
| strategy = 2, | |
| tolerance = 0.01, | |
| ) | |
| minuit = Minuit( | |
| pars -> nll(constructor, data, pars), | |
| start; | |
| names = string.(keys(start)), | |
| limits = collect(zip(lower, upper)), | |
| error = collect(step), | |
| arraycall = true, | |
| errordef = 0.5, | |
| strategy = 2, | |
| tolerance = 0.01, | |
| ) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #36 +/- ##
===========================================
+ Coverage 50.36% 86.83% +36.46%
===========================================
Files 9 11 +2
Lines 407 433 +26
===========================================
+ Hits 205 376 +171
+ Misses 202 57 -145 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Show the Optimization.jl MigradOptimizer path with ComponentArray bounds and document the lower-level Minuit step-size hook for descriptor uncertainties. Co-authored-by: Cursor <cursoragent@cursor.com>
61b34c8 to
f7db594
Compare
Summary
Optimization.jlOptimizationProblem/MigradOptimizerworkflow.problem_settings = (lb = lower, ub = upper).Minuitexample for passing descriptor uncertainties as Minuit step sizes viaerror = collect(step).Test plan
Made with Cursor