TopEFT / topeft

15 stars 24 forks source link

Make renormalization & factorization independent per-process systematics #327

Closed Andrew42 closed 1 year ago

Andrew42 commented 1 year ago

This PR adds a hardcoded check for the systematics renorm and fact and when it finds them it modifies the systematic name from renorm to renorm_PROCESS and similarly for the corresponding template histogram name PROCESS_renormUp to PROCESS_renorm_PROCESSUp.

For the processes that are split by EFT contribution, all decomposed terms will share a single nuisance parameter. So for example ttll may be split into ttll_sm,ttll_lin_cpt, ttll_quad_cpt, etc. There will be only one nuisance correlated over each of them (e.g. renorm_ttll or fact_ttll).

This PR also adds a new command-line option --drop-syst which can be used to specify one (or more) systematics that should be dropped from the pkl file before proceeding. Systematics that are dropped this way should match exactly what name of the systematic, however, the "Up"/"Down" postfix isn't needed as the code will remove both, unless you explicitly specify "SYSTNAMEUp", which will only drop the "Up" version of SYSTNAME. You can specify multiple systematics to drop at once by separating each name with by a space after the --drop-syst option.

codecov[bot] commented 1 year ago

Codecov Report

Merging #327 (a28051c) into master (3b78eb0) will decrease coverage by 0.03%. The diff coverage is 15.38%.

@@            Coverage Diff             @@
##           master     #327      +/-   ##
==========================================
- Coverage   30.45%   30.41%   -0.04%     
==========================================
  Files          47       47              
  Lines        7538     7561      +23     
==========================================
+ Hits         2296     2300       +4     
- Misses       5242     5261      +19     
Flag Coverage Δ
unittests 30.41% <15.38%> (-0.04%) :arrow_down:

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

Impacted Files Coverage Δ
analysis/topEFT/topeft.py 9.42% <ø> (+0.02%) :arrow_up:
topcoffea/modules/datacard_tools.py 69.33% <9.09%> (-1.72%) :arrow_down:
analysis/topEFT/make_cards.py 58.23% <50.00%> (-0.20%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

kmohrman commented 1 year ago

As discussed on MM and in the meetings, these changes (treating renorm and fact as independent systematics and and de correlating across processes) have been tested and the effects on the 2 sigma asimov profiled limits are very small (~1%). And we have agreed with the ARC and conveners to move to this new method of handing the renorm/fact systematics, so I will go ahead and merge the PR.

kmohrman commented 1 year ago

Since we're treating renorm and fact independently, we no longer need to include the variation where they are fluctuated together.

Consequently, I have now updated the processor to no longer include the renormfact variations (i.e. the variations where renorm and fact are varied together). I have also updated the run script wrapper (fullR2_run.sh) to no longer pass the --do-renormfact-envelope option to the run script.

@bryates I'm wondering if you could check that these changes look ok before we merge?

bryates commented 1 year ago

Since we're treating renorm and fact independently, we no longer need to include the variation where they are fluctuated together.

Consequently, I have now updated the processor to no longer include the renormfact variations (i.e. the variations where renorm and fact are varied together). I have also updated the run script wrapper (fullR2_run.sh) to no longer pass the --do-renormfact-envelope option to the run script.

@bryates I'm wondering if you could check that these changes look ok before we merge?

Yes, your changes all look good to me.

kmohrman commented 1 year ago

Since we're treating renorm and fact independently, we no longer need to include the variation where they are fluctuated together. Consequently, I have now updated the processor to no longer include the renormfact variations (i.e. the variations where renorm and fact are varied together). I have also updated the run script wrapper (fullR2_run.sh) to no longer pass the --do-renormfact-envelope option to the run script. @bryates I'm wondering if you could check that these changes look ok before we merge?

Yes, your changes all look good to me.

Ok, thanks for checking. Once the CI finishes, I'll go ahead and merge.