Type of suggested bounds changes when specified in model config

[alexander-held]

Summary

The per-parameter bounds from model.config.suggested_bounds() are tuples by default, but become lists when specified in the specification via the model config section. For consistency, it would be useful to stick with tuples. Happy to help look into and harmonize this.

OS / Environment

n/a

Steps to Reproduce

import pyhf

spec = {
    "channels": [
        {
            "name": "SR",
            "samples": [
                {
                    "data": [51.8],
                    "modifiers": [
                        {"data": None, "name": "mu", "type": "normfactor"},
                        {"data": [2.6], "name": "staterror_SR", "type": "staterror"},
                    ],
                    "name": "Signal",
                }
            ],
        }
    ],
    "measurements": [
        {
            "config": {
                "parameters": [{"name": "staterror_SR", "fixed": True, "inits": [1.1]}],
                "poi": "mu",
            },
            "name": "example",
        }
    ],
    "observations": [{"data": [52], "name": "SR"}],
    "version": "1.0.0",
}

model = pyhf.Workspace(spec).model()
print(model.config.suggested_bounds())

spec["measurements"][0]["config"]["parameters"].append({"name": "mu", "bounds": [[0, 10]]})
model = pyhf.Workspace(spec).model()
print(model.config.suggested_bounds())

output:

[(0, 10), (1e-10, 10.0)]
[[0, 10], (1e-10, 10.0)]

File Upload (optional)

No response

Expected Results

A List[Tuple[float, float]] in both cases: [(0, 10), (1e-10, 10.0)].

Actual Results

Specifying the bounds manually changes that part of the list from a tuple to a list.

[[0, 10], (1e-10, 10.0)]

pyhf Version

0.6.3 / master

Code of Conduct

• I agree to follow the Code of Conduct

[alexander-held]

I believe a change could maybe be introduced here

pyhf/src/pyhf/pdf.py

Lines 29 to 41 in 270d59d

	def _finalize_parameters_specs(user_parameters, _paramsets_requirements):
	# build up a dictionary of the parameter configurations provided by the user
	_paramsets_user_configs = {}
	for parameter in user_parameters:
	if parameter['name'] in _paramsets_user_configs:
	raise exceptions.InvalidModel(
	f"Multiple parameter configurations for {parameter['name']} were found."
	)
	_paramsets_user_configs[parameter.get('name')] = parameter
	_reqs = reduce_paramsets_requirements(
	_paramsets_requirements, _paramsets_user_configs
	)
	return _reqs

or in

pyhf/src/pyhf/parameters/utils.py

Line 10 in 270d59d

def reduce_paramsets_requirements(paramsets_requirements, paramsets_user_configs):

An addition of

if parameter.get("bounds"):
    parameter["bounds"] = [tuple(parameter["bounds"][0])]

before the reduce_paramsets_requirements call works for the example above.

[kratsg]

JSON doesn't know about tuples vs lists fyi. But in your code example, what happens with

spec["measurements"][0]["config"]["parameters"].append({"name": "mu", "bounds": [(0, 10)]})

instead? You are appending bounds as a list.. and as far as the code is aware/concerned, that's not an issue?

Ok, maybe your description is not clear - but is the issue that specifying the bounds or changing it -- from the default -- is what causes it to be injected as a list, even if you specify a tuple? but you didn't specify a tuple here.

EDIT: I see. JSON schema doesn't know how to treat tuples in python (because they're not really arrays) which is why the bounds come out like this.

Edit 2: see python-jsonschema/jsonschema#148

[alexander-held]

Sorry if the example was misleading, when appending {"name": "mu", "bounds": [(0, 10)]} the validation will fail. So from that perspective, and also from the perspective of starting out with a JSON that already contains the bounds, this tuple approach does not work. The only reason I appended in the example is to be able to show the difference in behavior.

specifying the bounds or changing it -- from the default -- is what causes it to be injected as a list

Yes, as soon as I manually specify the bounds in the measurement config (independent of whether or not the numbers I specify are the defaults anyway), the specific entry will be a list.

I caught this during dynamic type checking. Having uniform behavior would make typing a bit cleaner.

[kratsg]

The solution is probably to have type-checking to only specify the bounds as tuples I think (from the python code) but then also convert the bounds loaded from the JSON file to tuples... Need to think more because this breaks some minor portion of the code.