Skip to content

Commit

Permalink
fix: Correct handling of strategy 0 for Minuit optimizer (#2277)
Browse files Browse the repository at this point in the history
* Properly guard the `strategy` option for the Minuit optimizer in the case that
  `strategy` option of 0 is used.
  Without checking `self.strategy is not None` the `if self.strategy` for
  self.strategy==0 evaluates to truthiness of False, giving the wrong value of
  `not do_grad` for do_grad of False.
   - Amends PR #1183
* Use the corrected behavior to improve testing of default in test_minuit_strategy_global.
* Add Daniel Werner to contributors list.
  • Loading branch information
wernerd-cern authored Aug 15, 2023
1 parent f8f3c96 commit b81d903
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 3 deletions.
1 change: 1 addition & 0 deletions docs/contributors.rst
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,4 @@ Contributors include:
- Jerry Ling
- Nathan Simpson
- Beojan Stanislaus
- Daniel Werner
2 changes: 1 addition & 1 deletion src/pyhf/optimize/opt_minuit.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def _minimize(
# 0: Fast, user-provided gradient
# 1: Default, no user-provided gradient
strategy = options.pop(
'strategy', self.strategy if self.strategy else not do_grad
'strategy', self.strategy if self.strategy is not None else not do_grad
)
tolerance = options.pop('tolerance', self.tolerance)
if options:
Expand Down
3 changes: 1 addition & 2 deletions tests/test_optim.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,9 @@ def test_minuit_strategy_global(mocker, backend, strategy):
m = pyhf.simplemodels.uncorrelated_background([50.0], [100.0], [10.0])
data = pyhf.tensorlib.astensor([125.0] + m.config.auxdata)

do_grad = pyhf.tensorlib.default_do_grad
pyhf.infer.mle.fit(data, m)
assert spy.call_count == 1
assert spy.spy_return.minuit.strategy == strategy if do_grad else 1
assert spy.spy_return.minuit.strategy == strategy

pyhf.infer.mle.fit(data, m, strategy=0)
assert spy.call_count == 2
Expand Down

0 comments on commit b81d903

Please sign in to comment.