Skip to content

Commit

Permalink
document setting things to zero, and add pointers to the minuit issue…
Browse files Browse the repository at this point in the history
… that references this behaviour being done
  • Loading branch information
phinate committed Oct 20, 2023
1 parent 3a22760 commit 8f286ba
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 6 deletions.
13 changes: 8 additions & 5 deletions src/pyhf/optimize/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,16 @@ def _internal_postprocess(
# 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
# Set uncertainties for fixed parameters to 0 manually
if fixed_vals is not None: # check for fixed vals
if using_minuit:
# See related discussion here:
# 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)
else:
# Not using minuit, so don't have `fitresult.minuit.fixed` here: do it manually
fixed_bools = [False] * len(init_pars)
for index, _ in fixed_vals:
fixed_bools[index] = True
Expand All @@ -128,7 +130,8 @@ def _internal_postprocess(
cov = hess_inv

# we also need to edit the covariance matrix to zero-out uncertainties!
if fixed_vals is not None and not using_minuit: # minuit already does this
# NOTE: minuit already does this (https://github.com/scikit-hep/iminuit/issues/762#issuecomment-1207436406)
if fixed_vals is not None and not using_minuit:
fixed_bools = [False] * len(init_pars)
# Convert fixed_bools to a numpy array and reshape to make it a column vector
fixed_mask = tensorlib.reshape(
Expand Down
3 changes: 2 additions & 1 deletion tests/test_optim.py
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,8 @@ def test_optim_uncerts_autodiff(backend, source, spec, mu):
return_uncertainties=True,
)
assert result.shape == (2, 2)
# TODO: this does not match minuit at all (0.26418431) -- is that correct here?
# NOTE: this does not match minuit at all (0.26418431), and is probably an artefact of doing a fixed POI fit on this particular model?
# see discussion in https://github.com/scikit-hep/pyhf/pull/2269 for more details. differences disappear in global fit uncertainties.
assert pytest.approx([0.6693815171034548, 0.0]) == pyhf.tensorlib.tolist(
result[:, 1]
)
Expand Down

0 comments on commit 8f286ba

Please sign in to comment.