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

change to standard NLL fit objective #1006

Open
lukasheinrich opened this issue Jul 29, 2020 · 4 comments · May be fixed by #1057
Open

change to standard NLL fit objective #1006

lukasheinrich opened this issue Jul 29, 2020 · 4 comments · May be fixed by #1057
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed / contributions welcome

Comments

@lukasheinrich
Copy link
Contributor

lukasheinrich commented Jul 29, 2020

right now we use "twice_nll" as a fit objective and in the test statistic a simple diffence

twice_nll_constrfit - twice_nll_globalfit

but rather we should just to a NLL fit and in the test stat do

2*(nll_constrfit - nll_globalfit)

this will require updating some test reference numbers in the tests

@lukasheinrich lukasheinrich added good first issue Good for newcomers help wanted Extra attention is needed / contributions welcome labels Jul 29, 2020
@matthewfeickert
Copy link
Member

@lukasheinrich while I'm not against this, what is the primary motivation for the change?

@lukasheinrich
Copy link
Contributor Author

it's more customary to fit the NLL and I think we might have slight bus re errordef in minuit because we're fitting a scaled objective

@matthewfeickert
Copy link
Member

I think we might have slight bus re errordef in minuit because we're fitting a scaled objective

Good enough reason for me. 👍

@matthewfeickert matthewfeickert self-assigned this Sep 9, 2020
@alexander-held
Copy link
Member

alexander-held commented Mar 3, 2023

I just came across this again, but the errordef being used is fine, so no bugs there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed / contributions welcome
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants