Skip to content

Commit

Permalink
fix: Manually set uncertainties for fixed parameters to 0 (#1919)
Browse files Browse the repository at this point in the history
* Manually set the uncertainties for fixed parameters to zero after minimization,
and remove setting the uncertainty/step_sizes during minimization, to avoid warnings
from iminuit. For iminuit v2.12.2+ "assigning zero to errors is invalid, but in the
past this was not caught." This approach harmonizes with cabinetry for fixed
parameters for iminuit v2.12.2+.
   - c.f. scikit-hep/iminuit#762
   - c.f. scikit-hep/cabinetry#346
   - c.f. https://iminuit.readthedocs.io/en/stable/changelog.html#july-15-2022
     > fix a bug in error heuristic when parameters have negative values and prevent
     > assigning negative values to errors
* Remove tests/test_optim.py test_step_sizes_fixed_parameters_minuit as the
uncertainties/step sizes values are no longer set during minimization and so are no
longer in pyhf's control.
  • Loading branch information
matthewfeickert authored Aug 9, 2022
1 parent a4d5154 commit 355982a
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 20 deletions.
15 changes: 12 additions & 3 deletions src/pyhf/optimize/mixins.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
"""Helper Classes for use of automatic differentiation."""
from pyhf.tensor.manager import get_backend
import logging

import numpy as np

from pyhf import exceptions
from pyhf.optimize.common import shim

import logging
from pyhf.tensor.manager import get_backend

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -82,6 +84,13 @@ def _internal_postprocess(self, fitresult, stitch_pars, return_uncertainties=Fal
if uncertainties is not None:
# extract number of fixed parameters
num_fixed_pars = len(fitted_pars) - len(fitresult.x)

# FIXME: Set uncertainties for fixed parameters to 0 manually
# https://github.com/scikit-hep/iminuit/issues/762
# https://github.com/scikit-hep/pyhf/issues/1918
# https://github.com/scikit-hep/cabinetry/pull/346
uncertainties = np.where(fitresult.minuit.fixed, 0.0, uncertainties)

# stitch in zero-uncertainty for fixed values
uncertainties = stitch_pars(
tensorlib.astensor(uncertainties),
Expand Down
3 changes: 0 additions & 3 deletions src/pyhf/optimize/opt_minuit.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,12 @@ def _get_minimizer(
par_names=None,
):

step_sizes = [(b[1] - b[0]) / float(self.steps) for b in init_bounds]
fixed_vals = fixed_vals or []
# Minuit wants True/False for each parameter
fixed_bools = [False] * len(init_pars)
for index, val in fixed_vals:
fixed_bools[index] = True
init_pars[index] = val
step_sizes[index] = 0.0

# Minuit requires jac=callable
if do_grad:
Expand All @@ -68,7 +66,6 @@ def _get_minimizer(
jac = None

minuit = iminuit.Minuit(wrapped_objective, init_pars, grad=jac, name=par_names)
minuit.errors = step_sizes
minuit.limits = init_bounds
minuit.fixed = fixed_bools
minuit.print_level = self.verbose
Expand Down
14 changes: 0 additions & 14 deletions tests/test_optim.py
Original file line number Diff line number Diff line change
Expand Up @@ -520,20 +520,6 @@ def test_init_pars_sync_fixed_values_minuit(mocker):
assert minimizer.fixed == [True, False, False]


def test_step_sizes_fixed_parameters_minuit(mocker):
opt = pyhf.optimize.minuit_optimizer()

# patch all we need
from pyhf.optimize import opt_minuit

minuit = mocker.patch.object(getattr(opt_minuit, 'iminuit'), 'Minuit')
minimizer = opt._get_minimizer(None, [9, 9, 9], [(0, 10)] * 3, fixed_vals=[(0, 1)])

assert minuit.called
assert minimizer.fixed == [True, False, False]
assert minimizer.errors == [0.0, 0.01, 0.01]


def test_solver_options_behavior_scipy(mocker):
opt = pyhf.optimize.scipy_optimizer(solver_options={'arbitrary_option': 'foobar'})

Expand Down

0 comments on commit 355982a

Please sign in to comment.