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

feat: Configurable optimizer tolerance for termination #1184

Merged
merged 5 commits into from
Nov 19, 2020

Conversation

kratsg
Copy link
Contributor

@kratsg kratsg commented Nov 19, 2020

Pull Request Description

This PR fully resolves #929.

/cc @alexander-held

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 configurable optimizer tolerance for minimization termination
   - NB: SciPy defaults to None, iminuit defaults to 0.1
* Add tests to ensure the API for SciPy/iminuit

@kratsg kratsg added API Changes the public API feat/enhancement New feature or request optimization labels Nov 19, 2020
@kratsg kratsg self-assigned this Nov 19, 2020
@codecov
Copy link

codecov bot commented Nov 19, 2020

Codecov Report

Merging #1184 (b919fb4) into master (2010b6d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1184   +/-   ##
=======================================
  Coverage   97.43%   97.44%           
=======================================
  Files          63       63           
  Lines        3669     3674    +5     
  Branches      522      522           
=======================================
+ Hits         3575     3580    +5     
  Misses         55       55           
  Partials       39       39           
Flag Coverage Δ
unittests 97.44% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/pyhf/optimize/opt_minuit.py 100.00% <100.00%> (ø)
src/pyhf/optimize/opt_scipy.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2010b6d...b919fb4. Read the comment docs.

@matthewfeickert matthewfeickert added the tests pytest label Nov 19, 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.

LGTM. Thanks @kratsg

@matthewfeickert matthewfeickert merged commit 07fc449 into master Nov 19, 2020
@matthewfeickert matthewfeickert deleted the feat/optimizerTolerance branch November 19, 2020 04:02
Aspyona pushed a commit to Aspyona/pyhf that referenced this pull request Nov 22, 2020
* Add configurable optimizer tolerance for minimization termination
   - NB: SciPy defaults to None, iminuit defaults to 0.1
* Add tests to ensure the API for SciPy/iminuit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Changes the public API feat/enhancement New feature or request optimization tests pytest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose minuit instance in fit
2 participants