diff --git a/src/pyhf/optimize/mixins.py b/src/pyhf/optimize/mixins.py index 3d78af7cce..6f7adff518 100644 --- a/src/pyhf/optimize/mixins.py +++ b/src/pyhf/optimize/mixins.py @@ -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 @@ -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( diff --git a/tests/test_optim.py b/tests/test_optim.py index 5e0ce9338b..36032ac090 100644 --- a/tests/test_optim.py +++ b/tests/test_optim.py @@ -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] )