-
Notifications
You must be signed in to change notification settings - Fork 21
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
Using Scipy optimiser (for ranking and limits) #413
Comments
Hi @MarcelHoh, that sounds like a useful addition. We could add a |
I can give it a go. To add some data, with scipy a test fit is taking me 40ms to 12s with minuit. For a ranking with ~190 parameters, as I'm using for a test, this ends up being 6 minutes (tested) with scipy to ~3+ hours with minuit (estimated). |
I am not familiar with the code (this is my first real look at it) but I would naively lean towards adding a |
That's an impressively big difference in runtime between minimizers! I don't think I have seen one that is quite this striking before. I would prefer to stick with Minuit as the default minimizer regardless. It is often slower and can be more fragile, but I have seen a number of cases where the scipy minimizer appears to have converged but returns nonsensical results. I have a lot more confidence in the minimum being accurate when determined via Minuit and think the safe but slow choice is therefore the better default. This is a place where |
In my experience using scipy as the minimiser is significantly faster than minuit. This can be very useful when the uncertainty of each parameter is not needed such as limit setting and ranking parameters.
Within cabinetry the
cabinetry.fit.._fit_model
function always overrides the set minimizer in pyhf to be minuit. This makes the ranking and limits quite a bit slower than they need to be. For the ranking only the first fit, with no fixed nuisance parameters, should need to use minuit as the optimiser.Would it be possible to make the default switch to minuit an option in
cabinetry.fit._fit_model
? This would allow for always using minuit if the user desires but also to switch to scipy for all cases where the uncertainty is not needed.The text was updated successfully, but these errors were encountered: