`lumi` modifier documentation and behavior

[alexander-held]

Question

The lumi modifier is described in the documentation with a reference to \sigma_\lambda and the measurement section (I believe the link to that may be slightly broken, it does not lead exactly to the measurement section).

The measurement section shows an example

{ "name":"lumi", "auxdata":[1.0],"sigmas":[0.017], "bounds":[[0.915,1.085]],"inits":[1.0] },

for configuring a lumi modifier, but does not describe which of these properties are optional. Here is an example setup for the following discussion:

import pyhf

spec = {
    "channels": [
        {
            "name": "Signal Region",
            "samples": [
                {
                    "data": [35],
                    "modifiers": [
                        {
                            "data": None,
                            "name": "Signal strength",
                            "type": "normfactor",
                        },
                        {"data": None, "name": "lumi", "type": "lumi"},
                    ],
                    "name": "Signal",
                }
            ],
        }
    ],
    "measurements": [
        {
            "config": {
                "parameters": [
                    {
                        "auxdata": [1.0],
                        "bounds": [[0.9, 1.1]],
                        "inits": [1.0],
                        "name": "lumi",
                        "sigmas": [0.02],
                    },
                ],
                "poi": "Signal strength",
            },
            "name": "lumi",
        },
    ],
    "observations": [{"data": [35], "name": "Signal Region"}],
    "version": "1.0.0",
}

ws = pyhf.Workspace(spec)
model = ws.model()

The auxdata field naively seems superfluous, and I would expect it to default to 1.0. This is not the case, removing it results in the following traceback:

Traceback (most recent call last):
  File "test.py", line 45, in <module>
    model = ws.model()
  File "[...]/pyhf/src/pyhf/workspace.py", line 418, in model
    return Model(modelspec, poi_name=measurement['config']['poi'], **config_kwargs)
  File "[...]/pyhf/src/pyhf/pdf.py", line 674, in __init__
    self.config.auxdata += parset.auxdata
TypeError: 'NoneType' object is not iterable

It might be useful to either catch this via the schema (might not be possible, since the parameter config item does not know what modifier it refers to), or dynamically. I also would suggest defaulting to an auxdata value of 1.0, since for all other modifier types this is an optional property.

Possibly even more surprising is that inits is not optional either:

Traceback (most recent call last):
  File "test.py", line 45, in <module>
    model = ws.model()
  File "[...]/pyhf/src/pyhf/workspace.py", line 418, in model
    return Model(modelspec, poi_name=measurement['config']['poi'], **config_kwargs)
  File "[...]/pyhf/src/pyhf/pdf.py", line 657, in __init__
    self.config = _ModelConfig(spec, **config_kwargs)
  File "[...]/pyhf/src/pyhf/pdf.py", line 261, in __init__
    self.npars = len(self.suggested_init())
  File "[...]/pyhf/src/pyhf/pdf.py", line 281, in suggested_init
    init = init + self.par_map[name]['paramset'].suggested_init
TypeError: can only concatenate list (not "NoneType") to list

This also differs from the usual case of other modifiers where this is optional.

The bounds are optional, which seems reasonable to me. The possibly biggest surprise to me is that "sigmas" is optional, the code runs fine when removing this. I would argue that this is the one property (besides the name) that is absolutely needed, since a reasonable default cannot be determined.

Summary:

• Could auxdata/ inits for lumi modifiers become optional, and possibly be caught dynamically with a dedicated error message if not optional and missing?
• Why is sigmas not a required parameter? If a default is set for that property, presumably all the remaining settings of the lumi parameter would have to be set accordingly and then the whole config for this parameter should become optional (at the moment it is required).

Relevant Issues and Pull Requests

none I know of

[alexander-held]

As described in the HistFactory documentation in section 3.2, there are two conventions user may follow, resulting in auxdata=1 or auxdata equal to the luminosity. This may be sufficient motivation to force the user to explicitly specify auxdata, but assuming 1 as default also seems reasonable to me. In either case, from my understanding the auxdata itself is a reasonable default for the initial value, and the sigmas setting is essential for the modifier to be meaningful (it seems to default to 1 aka 100% luminosity uncertainty, but model.config.param_set("lumi").sigmas does not get set when it is not in the measurement config, so this may be an accidental setting?).

[kratsg]

I think one issue is that auxdata is required for any modifier that is constrained - and lumi is a constrained modifier. We could expand the schema to be more specific about which is required/optional for each modifier specifically, but this might need a deeper dive into the pyhf code to make sure we don't break anything.

[MoAly98]

I ran into a similar issue with parameter bounds for a lumi modifier. The spec I am using is the following:

    spec = {
        "channels": [
            {
                "name": "SR",
                "samples": [
                    {
                        "data": [35.0, 30.0],
                        "modifiers": [
                            {"data": None, "name": "mu", "type": "normfactor"},
                            {
                                "data": None,
                                "name": "mu_shapefactor",
                                "type": "shapefactor",
                            },
                            {
                                "data": {"hi": 0.8, "lo": 1.2},
                                "name": "normsys",
                                "type": "normsys",
                            },
                            {
                                "data": {
                                    "hi_data": [36.0, 31.0],
                                    "lo_data": [34.0, 29.0],
                                },
                                "name": "histosys",
                                "type": "histosys",
                            },
                            {
                                "data": [1.0, 2.0],
                                "name": "staterror_SR",
                                "type": "staterror",
                            },
                            {
                                "data": [1.0, 2.0],
                                "name": "shapesys_SR",
                                "type": "shapesys",
                            },
                            {"data": None, "name": "lumi", "type": "lumi"},
                        ],
                        "name": "Signal",
                    }
                ],
            }
        ],
        "measurements": [
            {
                "config": {
                    "parameters": [
                        {
                            "auxdata": [1.0],
                            "inits": [1.0],
                            "name": "lumi",
                            "sigmas": [0.02],
                        }
                    ],
                    "poi": "mu",
                },
                "name": "lumi modifier",
            }
        ],
        "observations": [{"data": [35, 30], "name": "SR"}],
        "version": "1.0.0",
    }

The following code crashes:

model, data = model_utils.model_and_data(spec)
model.config.suggested_bounds()

with the error:

File /opt/anaconda3/envs/cabinetry/lib/python3.12/site-packages/pyhf/pdf.py:340, in _ModelConfig.suggested_bounds(self)
    338 bounds = []
    339 for name in self.par_order:
--> 340     bounds = bounds + self.par_map[name]['paramset'].suggested_bounds
    341 return bounds

TypeError: can only concatenate list (not "NoneType") to list

As @alexander-held pointed out, either spec validation should catch this and require the presence of these extra arguments or there should be a sensible default for them.

I am happy to have a look into the pyhf code, but any hints on where simply modifying the suggested_xxx methods can cause things to break would be appreciated.

[alexander-held]

I think we need to figure out what the required parameters have to be and then perform a few changes accordingly:

• sigmas seems mandatory to me always (but is not currently),
• auxdata could default to 1 but it seems reasonable for me to make it required (which it is already),
• inits can be derived from auxdata to get a default value (currently not optional),
• bounds can be derived knowing the above as something like auxdata +/- 5 sigmas I think (currently optional during validation but crashes at runtime).

I am overall also in the camp of recommending the use of normsys instead of lumi. It provides more flexibility for correlation and lacks the auxdata!=1 configuration but that could be implemented with a constant normfactor of appropriate value as well.