Skip to content
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

Adding new options for roofit: SetMaxIter(), SetMaxCalls() and SetEpsilon() #137

Closed
wants to merge 2 commits into from

Conversation

FableQuentin
Copy link

Here are the modifications I have submitted in the "Stat and Math ROOT Support" forum in order to add new options in RooAbsPdf::FitTo() options. The three options are:
-SetMaxIter(Int_t max) in order to modify the maximum number of iterations for the fit
-SetMaxCall(Int_t max) in order to modify the maximum number of calls for the fit
-SetEpsilon(Int_t eps) in order to modify the epsilon value of the fit

Best regards,
Quentin Fable, PhD student at GANIL.

@lmoneta
Copy link
Member

lmoneta commented Mar 16, 2016

Hi,

I am sorry I cannot include the patch as it is in RooFit because it has the following problems:

  • the comments are re-formatted to the old THTML style. They should be mantained to the Doxygen style. Look at the diff to understand it
  • The patch is also not correct in case of RooMinuit for the SetEps option. This is due to a naming problem already existing in RooMinimizer. The value you want to change I guess is not the eps (the floating point precision) but the minimization tolerance.
    The EPS parameter in Minuit controls the precision the function is computed and it should be kept, apart n some exceptional case to the normal double precision value. The parameter you want to change is instead the tolerance for the minimisation, which by default is set to 1 in RooFit.

So I would change the option to SetTolerance or SetTol and implement it correctly in RooMinuit. In RooMinimizer unfortunately the method setting the tolerance is called SetEps. Probably this method should be changed

Please let me know if you can make these changes,
Thank you

Lorenzo

@peremato peremato assigned lmoneta and unassigned lmoneta Mar 1, 2017
@vgvassilev
Copy link
Member

Enabling clang-format feedback.

@vgvassilev vgvassilev closed this Mar 28, 2017
@vgvassilev vgvassilev reopened this Mar 28, 2017
@phsft-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@lmoneta
Copy link
Member

lmoneta commented Apr 12, 2017

Hi,

Can you apply the changes as suggested in my previous comment one year ago ?

Thanks for letting me know

Best Regards

Lorenzo

@lmoneta
Copy link
Member

lmoneta commented Apr 13, 2017

Since there are no reply I close now this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants