Repository: scikit-hep/cabinetry

Issue:
Handle partially fixed parameters in `model_utils` API
The support for partially fixed parameters (such as `staterror` with zeros in some bins) in `pyhf` 0.7 can result in scenarios where the behavior of `suggested_fixed_as_bool` is undefined, see https://github.com/scikit-hep/pyhf/issues/1944.

This currently breaks `model_utils.prefit_uncertainties` in `cabinetry` (and also affects `model_utils.unconstrained_parameter_count`). Working around this should be possible by iterating bin by bin for affected parameters and using the `suggested_fixed` API instead.

Discussion:
codecov[bot]: ## [Codecov](https://codecov.io/gh/scikit-hep/cabinetry/pull/396?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) Report
Patch coverage: **`100.00`**% and no project coverage change.
> Comparison is base [(`bab591c`)](https://codecov.io/gh/scikit-hep/cabinetry/commit/bab591cf03672066529155138d0e63d58f924114?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) 100.00% compared to head [(`071e330`)](https://codecov.io/gh/scikit-hep/cabinetry/pull/396?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) 100.00%.

> :exclamation: Current head 071e330 differs from pull request most recent head 979b538. Consider uploading reports for the commit 979b538 to get more accurate results

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff            @@
##            master      #396   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           23        23           
  Lines         2046      2047    +1     
  Branches       324       325    +1     
=========================================
+ Hits          2046      2047    +1     
```


| [Impacted Files](https://codecov.io/gh/scikit-hep/cabinetry/pull/396?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) | Coverage Δ | |
|---|---|---|
| [src/cabinetry/model\_utils.py](https://codecov.io/gh/scikit-hep/cabinetry/pull/396?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep#diff-c3JjL2NhYmluZXRyeS9tb2RlbF91dGlscy5weQ==) | `100.00% <100.00%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep)

</details>

[:umbrella: View full report in Codecov by Sentry](https://codecov.io/gh/scikit-hep/cabinetry/pull/396?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep).


Patch:
diff --git a/src/cabinetry/model_utils.py b/src/cabinetry/model_utils.py
index cc6285e..f336e60 100644
--- a/src/cabinetry/model_utils.py
+++ b/src/cabinetry/model_utils.py
@@ -179,19 +179,19 @@ def prefit_uncertainties(model: pyhf.pdf.Model) -> np.ndarray:
     """
     pre_fit_unc = []  # pre-fit uncertainties for parameters
     for parameter in model.config.par_order:
-        # obtain pre-fit uncertainty for constrained, non-fixed parameters
-        if (
-            model.config.param_set(parameter).constrained
-            and not model.config.param_set(parameter).suggested_fixed_as_bool
-        ):
-            pre_fit_unc += model.config.param_set(parameter).width()
+        if model.config.param_set(parameter).constrained:
+            # pre-fit uncertainty for constrained parameters (if fixed, set to 0.0)
+            widths = [
+                width if not fixed else 0.0
+                for width, fixed in zip(
+                    model.config.param_set(parameter).width(),
+                    model.config.param_set(parameter).suggested_fixed,
+                )
+            ]
+            pre_fit_unc += widths
         else:
-            if model.config.param_set(parameter).n_parameters == 1:
-                # unconstrained normfactor or fixed parameter, uncertainty is 0
-                pre_fit_unc.append(0.0)
-            else:
-                # shapefactor
-                pre_fit_unc += [0.0] * model.config.param_set(parameter).n_parameters
+            # unconstrained: normfactor or shapefactor, uncertainty is 0
+            pre_fit_unc += [0.0] * model.config.param_set(parameter).n_parameters
     return np.asarray(pre_fit_unc)
 
 
@@ -476,11 +476,9 @@ def unconstrained_parameter_count(model: pyhf.pdf.Model) -> int:
     """
     n_pars = 0
     for parname in model.config.par_order:
-        if (
-            not model.config.param_set(parname).constrained
-            and not model.config.param_set(parname).suggested_fixed_as_bool
-        ):
-            n_pars += model.config.param_set(parname).n_parameters
+        if not model.config.param_set(parname).constrained:
+            # only consider non-constant parameters
+            n_pars += model.config.param_set(parname).suggested_fixed.count(False)
     return n_pars
 
 


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index aeb3cbf..eda236e 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -379,6 +379,43 @@ def example_spec_modifiers():
     return spec
 
 
+@pytest.fixture
+def example_spec_zero_staterror():
+    spec = {
+        "channels": [
+            {
+                "name": "SR",
+                "samples": [
+                    {
+                        "data": [5.0, 0.0],
+                        "modifiers": [
+                            {"data": None, "name": "mu", "type": "normfactor"},
+                            {
+                                "data": [1.0, 0.0],
+                                "name": "staterror_SR",
+                                "type": "staterror",
+                            },
+                        ],
+                        "name": "Signal",
+                    }
+                ],
+            }
+        ],
+        "measurements": [
+            {
+                "config": {
+                    "parameters": [],
+                    "poi": "mu",
+                },
+                "name": "zero staterror",
+            }
+        ],
+        "observations": [{"data": [5, 0], "name": "SR"}],
+        "version": "1.0.0",
+    }
+    return spec
+
+
 # code below allows marking tests as slow and adds --runslow to run them
 # implemented following https://docs.pytest.org/en/6.2.x/example/simple.html
 def pytest_addoption(parser):
diff --git a/tests/test_model_utils.py b/tests/test_model_utils.py
index 257a00d..b1369d6 100644
--- a/tests/test_model_utils.py
+++ b/tests/test_model_utils.py
@@ -147,7 +147,10 @@ def test_asimov_parameters(example_spec, example_spec_shapefactor, example_spec_
 
 
 def test_prefit_uncertainties(
-    example_spec, example_spec_multibin, example_spec_shapefactor
+    example_spec,
+    example_spec_multibin,
+    example_spec_shapefactor,
+    example_spec_zero_staterror,
 ):
     model = pyhf.Workspace(example_spec).model()
     unc = model_utils.prefit_uncertainties(model)
@@ -161,6 +164,10 @@ def test_prefit_uncertainties(
     unc = model_utils.prefit_uncertainties(model)
     assert np.allclose(unc, [0.0, 0.0, 0.0])
 
+    model = pyhf.Workspace(example_spec_zero_staterror).model()
+    unc = model_utils.prefit_uncertainties(model)
+    assert np.allclose(unc, [0.0, 0.2, 0.0])  # partially fixed staterror
+
 
 def test__hashable_model_key(example_spec):
     # key matches for two models built from the same spec