feat: Experimental implementation of custom modifiers

[kratsg]

Pull Request Description

Refer to #850.

Checklist Before Requesting Reviewer

• Tests are passing
• "WIP" removed from the title of the pull request
• Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

• Summarize commit messages into a comprehensive review of the PR

* Add `pyhf.experimental` module to allow for user defined custom modifiers
  using numexpr string expressions.
* Add 'experimental' extra with numexpr dependency.
* Add tests for `pyhf.experimental.modifiers`.
* Add `pyhf.experimental` to API docs.
* Add 'experimental' extra to Docker image build.

[codecov]

Codecov Report

Attention: Patch coverage is 88.63636% with 10 lines in your changes missing coverage. Please review.

Project coverage is 98.09%. Comparing base (ecbe778) to head (bb302c2).
Report is 106 commits behind head on main.

Files with missing lines	Patch %	Lines
src/pyhf/experimental/modifiers.py	88.63%	6 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1991      +/-   ##
==========================================
- Coverage   98.28%   98.09%   -0.19%     
==========================================
  Files          69       70       +1     
  Lines        4538     4626      +88     
  Branches      803      814      +11     
==========================================
+ Hits         4460     4538      +78     
- Misses         45       51       +6     
- Partials       33       37       +4     

Flag	Coverage Δ
contrib	97.68% <88.63%> (-0.18%)	⬇️
doctest	61.13% <77.27%> (+0.42%)	⬆️
unittests-3.10	96.15% <88.63%> (-0.15%)	⬇️
unittests-3.11	96.15% <88.63%> (-0.15%)	⬇️
unittests-3.8	96.17% <88.63%> (-0.15%)	⬇️
unittests-3.9	96.19% <88.63%> (-0.15%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[lukasheinrich]

do we want to differentiate between experimental and contrib?

[alexander-held]

My reading of "contrib" is "quite stable API, but might get factored out eventually", while I would view "experimental" more like "anything goes, expect things to break". You could of course communicate how exactly you want the modules to be understood, but this is what I would think without further context.

[matthewfeickert]

do we want to differentiate between experimental and contrib?

Yes, I think that's a good idea if we really mean "experimental". I have the same take as @alexander-held does.

[lukasheinrich]

Ok !

[alexander-held]

Related to the discussion in scikit-hep/cabinetry#382, it may be good to provide a suitable schema for this as well (perhaps same PR makes sense?).

[rmnmllr]

If I may add a request, it would be great if pyhf could catch a wrongly typed function expression in the custom modifier. I stumbled over srqt instead of sqrt in the related cabinetry issue discussion scikit-hep/cabinetry#382 (comment) which produced an obscure error output.

[matthewfeickert]

@kratsg while I will go back and review Issue #850 (I've forgotten all of it) can you also comment on what else would be needed for this PR before it would be ready for review?

[pariRieck]

Hi, just to comment also here that on the side of an ATLAS analysis there is some urgency with this and a merge would help us.
We are looking for a way to incorporate different signal histograms into our statistical model and they scale differently with the parameter of interest, hence custom_modifiers are what we are looking for.
Apart from that, we should be fine.

[jmontejo]

Hi,
just wanted to add a +1 of an ATLAS analysis (RPV-multijets) depending on this to leave HistFitter for pyhf.

Cheers
Javier

[jmontejo]

Hi,
just wanted to add a +1 of an ATLAS analysis (RPV-multijets) depending on this to leave HistFitter for pyhf.

Cheers
Javier

[matthewfeickert]

@kratsg I assume that these prints were put in here for debugging? and that we can remove them and they aren't actually supposed to be in there for logging. Can you confirm?

[matthewfeickert]

Suggested change

print(thismod)

[matthewfeickert]

I've removed the prints in bb302c2 but if we need them back for debugging quickly just rever it.

[matthewfeickert]

Relevant RTD build: https://pyhf--1991.org.readthedocs.build/en/1991/api.html#experimental

[alexander-held]

Small edge case: if you define a custom parameter to plug into add_custom_modifier and then define that parameter again in the "normal" way as a normfactor attached to a sample (like {"type": "normfactor", "name": "mu", "data": None} that seems to generally work but only as long as the bounds you put into the custom modifier version sync up with the bounds that the "normal" version would create by default. It probably would make sense to not add new bounds when the parameter is added as a normfactor.

[alexander-held]

While giving this another try, I ran into something that is either a bug or a usage question. I also have some API feedback. I opened #2350 for this to not clutter this PR too much.