Skip to content

Commit

Permalink
fix: Ensure _ModelConfig.suggested_fixed list contains only boolean…
Browse files Browse the repository at this point in the history
…s for all modifiers (#1706)

* Have the fixed parameters selection in staterror_builder be a list
of Bools over numpy.bool_
* Add type hints for the relevant sections
* Add test to test_modifiers.py to require the fixed parameters
selection list to be Bool
  • Loading branch information
alexander-held authored Nov 18, 2021
1 parent f25b8dd commit 9f9a6b4
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 5 deletions.
6 changes: 4 additions & 2 deletions src/pyhf/modifiers/staterror.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logging
from typing import List

import pyhf
from pyhf import events
Expand All @@ -8,7 +9,7 @@
log = logging.getLogger(__name__)


def required_parset(sigmas, fixed):
def required_parset(sigmas, fixed: List[bool]):
n_parameters = len(sigmas)
return {
'paramset_type': 'constrained_by_normal',
Expand Down Expand Up @@ -104,7 +105,8 @@ def finalize(self):
for modifier_data in self.builder_data[modname].values():
modifier_data['data']['mask'] = masks[modname]
sigmas = relerrs[masks[modname]]
fixed = [s == 0 for s in sigmas]
# list of bools, consistent with other modifiers (no numpy.bool_)
fixed = default_backend.tolist(sigmas == 0)
# ensures non-Nan constraint term, but in a future PR we need to remove constraints for these
sigmas[fixed] = 1.0
self.required_parsets.setdefault(parname, [required_parset(sigmas, fixed)])
Expand Down
6 changes: 4 additions & 2 deletions src/pyhf/parameters/paramsets.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from typing import List

import pyhf

__all__ = [
Expand Down Expand Up @@ -27,13 +29,13 @@ def __init__(self, **kwargs):
)

@property
def suggested_fixed(self):
def suggested_fixed(self) -> List[bool]:
if type(self._suggested_fixed) == bool:
return [self._suggested_fixed] * self.n_parameters
return self._suggested_fixed

@property
def suggested_fixed_as_bool(self):
def suggested_fixed_as_bool(self) -> bool:
'''compresses list of same-value bools into single bool'''
suggested_fixed = self.suggested_fixed
first = suggested_fixed[0]
Expand Down
3 changes: 2 additions & 1 deletion src/pyhf/pdf.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import copy
import logging
from typing import List

import pyhf.parameters
import pyhf
Expand Down Expand Up @@ -346,7 +347,7 @@ def param_set(self, name):
"""
return self.par_map[name]['paramset']

def suggested_fixed(self):
def suggested_fixed(self) -> List[bool]:
"""
Identify the fixed parameters in the model.
Expand Down
12 changes: 12 additions & 0 deletions tests/test_modifiers.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,24 @@ def test_staterror_holes():
True,
False,
]
assert all(
[
isinstance(fixed, bool)
for fixed in model.config.param_set("staterror_1").suggested_fixed
]
)
assert model.config.param_set("staterror_2").suggested_fixed == [
False,
True,
False,
False,
]
assert all(
[
isinstance(fixed, bool)
for fixed in model.config.param_set("staterror_2").suggested_fixed
]
)
assert (factors[1][0, 0, 0, :] == [2.0, 1.0, 1.0, 3.0, 1.0, 1.0, 1.0, 1.0]).all()
assert (factors[1][1, 0, 0, :] == [1.0, 1.0, 1.0, 1.0, 4.0, 1.0, 5.0, 6.0]).all()

Expand Down

0 comments on commit 9f9a6b4

Please sign in to comment.