Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accelerated interference models #842

Merged
merged 25 commits into from
Nov 6, 2023
Merged

Accelerated interference models #842

merged 25 commits into from
Nov 6, 2023

Conversation

nsmith-
Copy link
Collaborator

@nsmith- nsmith- commented Jun 13, 2023

For binned fits, the inability to have a negative template makes implementing certain interference models rather challenging. Previous approaches such as https://github.com/amassiro/AnalyticAnomalousCoupling work hard to rewrite the amplitude components to all be positive templates, which then can be used with CMSHistFunc or CMSHistSum (what you get with autoMCStats, and with --use-histsum, respectively) as if they were normal signal processes.

What I propose here is to directly modify the nominal yield prior to any nuisance parameter morphing directly inside CMSHistFunc/CMSHistSum. The test_interference.py shows an example setup.

Plan:

  • Implement CMSInterferenceFunc with matrix norm computation
  • Add injectExternalMorph to CMSHistFunc
  • Work out user interface for adding these morphs with text2workspace.py command
  • Add injectExternalMorph to CMSHistSum
  • Allow other external morph types besides CMSInterferenceFunc

For future PR

  • Implement another morph type to enable RooParametricHist-like functionality with autoMCStats
  • Experiment with more efficient computations for _InterferenceEval
  • Morph-aware MC stat uncertainty (for e.g. HH model)

@nsmith- nsmith- requested a review from ajgilbert June 13, 2023 16:18
@nsmith-
Copy link
Collaborator Author

nsmith- commented Aug 25, 2023

I'm thinking to leave some work for a future PR because what is here already is a significant (100x) performance improvement for the TOP-22-006 model.

@nsmith- nsmith- marked this pull request as ready for review August 25, 2023 19:56
@nsmith-
Copy link
Collaborator Author

nsmith- commented Aug 29, 2023

Thanks to @Andrew42 for pointing out that lower triangular storage makes more sense.

@jonathon-langford
Copy link
Collaborator

I'm in favour of merging this now and leaving the remaining items to a future PR. I would prioritise adding the functionality to CMSHistSum as this is what we use in the Higgs combination (which will likely be the first implementation of this model for EFT shape effects).

I had a few comments that I will leave here for archiving/discussion.

  1. Useful to add functionality to swap between inclusive and per-bin scaling if shape effects are small. Some combination of the InterferenceFunc and RooEFTScalingFunction?
  2. Clear documentation on how to construct the lower triangle of matrix for SMEFT.
  3. Impact of MC stat uncertainties. In HH model the stat uncertainties are well modelled for different points in parameter space by having multiple templates. This is not the case here. How can we account for MC stat uncertainty at non-SM points.
  4. Implement the matrix multiplication lib (Eigen) into RooEFTScalingFunction.

@anigamova
Copy link
Collaborator

I have been testing this PR with the datacards from HIG PAG, haven't found any issue, but I think making it work with the CMSHistSum is very important. Agree that we can merge this PR, but we probably should resolve the conflicts first

@nsmith-
Copy link
Collaborator Author

nsmith- commented Nov 1, 2023

Ok, conflicts removed and now CMSHistSum should work.

@anigamova
Copy link
Collaborator

Alright, happy to merge! (will wait until the end of the week in case of objections)
Thanks a lot @nsmith- :-)

@nsmith- nsmith- requested a review from anigamova November 1, 2023 19:04
@ajgilbert
Copy link
Collaborator

Looks good, 👍

@anigamova
Copy link
Collaborator

I checked with the likelihood scans that the limits with CMSHistSum and CMSHistFunc are compatible, happy to merge
image

@anigamova anigamova merged commit bed1eed into main Nov 6, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants