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

test: Verify NLL values and Minuit parameter uncertainties #1197

Merged

Conversation

alexander-held
Copy link
Member

@alexander-held alexander-held commented Nov 25, 2020

Description

Resolves #1193.

This adds a check for the NLL value returned by minimization with various backends, which is compared against a reference.

It also adds a check for the parameter uncertainties returned by Minuit, which was motivated by #1183 (the addition of the HESSE call would have resulted in a slight change in the uncertainties returned here). A test for the parameter values could also be added here, but since that behavior is tested in test_minimize already I did not add it again here.

I noticed that three functions set parameters constant without using the keyword argument fixed_vals, which I expect is not on purpose. I added another commit in here that changes that.

Checklist Before Requesting Reviewer

  • Tests are passing
  • "WIP" removed from the title of the pull request
  • Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

  • Summarize commit messages into a comprehensive review of the PR
* Add a test for the NLL value returned by minimization
* Explicitly specify the fixed_vals kwarg for function calls passing in fixed parameter values

@kratsg kratsg added refactor A code change that neither fixes a bug nor adds a feature tests pytest labels Nov 25, 2020
@kratsg kratsg self-assigned this Nov 25, 2020
Copy link
Member

@matthewfeickert matthewfeickert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks very much for the PR, @alexander-held! LGTM. :shipit:

@matthewfeickert matthewfeickert merged commit 0c8c0c4 into scikit-hep:master Nov 25, 2020
@alexander-held alexander-held deleted the test/verify-uncertainties-nll branch November 25, 2020 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor A code change that neither fixes a bug nor adds a feature tests pytest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expand test for parameter uncertainties
3 participants