`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.