cmssw
cmssw copied to clipboard
Implemented new modifiers for mvaTTH Run 3
PR description:
Brief description
Dear reviewers,
This PR is to introduce the newest training of the Muon mvaTTH in CMSSW. The training has been performed following what was done already done for Run 2, but using Run 3 MC samples for training and validation.
Context of the PR
The goal for this new training is to be used as a substitute of the current mvaTTH only in Run 3 MC production. That is: for Run 2 we still need to keep the current mvaTTH implementation (can be found in these lines), and for Run 3 we want to make use of an era modifier to fetch the proper --new -- weights that have been computed.
What this PR includes
This change should only affect Run 3 production. For that, this PR includes:
- The required era modifiers in
Configuration/Eras/
.- I've added two new era modifiers
run3_muon_2022
andrun3_muon_2023
, which are called from the generalRun3_Eras_cff.py
. - I hope this is the optimal way to handle our use case.
- I've added two new era modifiers
- The call to the era modifier and the implementation of the new mvaTTH evaluation.
- How the mva is evaluated changes a little bit with respect to the previous version: mainly the order in which the variables are parsed to TMVA and also we have added a new additional variable which is
LepGood_pfRelIso03_all
. - Which weights should be read in order to properly compute the mva score.
- How the mva is evaluated changes a little bit with respect to the previous version: mainly the order in which the variables are parsed to TMVA and also we have added a new additional variable which is
Validation
I've performed a local test running a simple cmsRun
script to see that the mva is loaded and nanoAOD is in fact produced.
Other comments
Please let me know if the proposal is reasonable, and if there's anything else I can do for testing. I'm adding the L2 muon convener @JanFSchulte so he can follow up the PR as well. Feel free to add any other muon convener (I don't have their github user names unfortunately).
Linked PRs
This is the link for the PR that includes the weights in CMS-data: pull16.
cms-bot internal usage
+code-checks
Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44322/39349
- This PR adds an extra 28KB to repository
A new Pull Request was created by @Cvico for master.
It involves the following packages:
- Configuration/Eras (operations)
- PhysicsTools/NanoAOD (xpog)
@davidlange6, @antoniovilela, @rappoccio, @vlimant, @fabiocos, @cmsbuild, @hqucms can you please review it and eventually sign? Thanks. @makortel, @gpetruc, @AnnikaStein, @missirol, @fabiocos, @Martin-Grunewald this is something you requested to watch as well. @antoniovilela, @rappoccio, @sextonkennedy you are the release manager for this.
cms-bot commands are listed here
I'd rather have the default be run3, and you revert to the old using ~run3
I'd rather have the default be run3, and you revert to the old using
~run3
Thanks @vlimant for the suggestion. I don't have a feeling on what's the best option here, but could you point me on how to properly do this?
Do I have to change these lines, and call the run2 modifier instead of the run3 modifier?
+code-checks
Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44322/39359
- This PR adds an extra 28KB to repository
Pull request #44322 was updated. @antoniovilela, @davidlange6, @vlimant, @fabiocos, @rappoccio, @cmsbuild, @hqucms can you please check and sign again.
Hi @Cvico From the POG side we would like to take the opportunity to rename the variable from mvaTTH to mvaPrompt to unify the naming with what is used in the MUO-22-001 paper and to not have it so closely associated with just one analysis since we want this to be an official POG product going forward. Could you take care of that? It should probably be mentioned in the doc string what the previous name was.
Hi @Cvico From the POG side we would like to take the opportunity to rename the variable from mvaTTH to mvaPrompt to unify the naming with what is used in the MUO-22-001 paper and to not have it so closely associated with just one analysis since we want this to be an official POG product going forward. Could you take care of that? It should probably be mentioned in the doc string what the previous name was.
Hi Jan. From my side I personally like the idea of changing the name to mvaPrompt (more precisely, I don't have strong opinion against it). But since there's a mvaTTH for electrons, I think EGM conveners should be aware of the change as well. I can quickly contact them with the proposal.
Thanks @Cvico, this further change should happen basically now to get everything in place.
Hi, I've updated the name from mvaTTH
to promptMVA
as @JanFSchulte suggested, also in the electrons_cff.py
config. I've checked it produces an output with two branches: Electron_promptMVA
and Muon_promptMVA
.
I've also included two changes: one in nano_eras_cff.py
to include the run3_muon
modifiers, and also I had to modify the muonMVALowPt to fetch the variables for the Run 2 training explicitly. The reason for this latter change is because it was previously working on TOP of the Run 2 mvaTTH configuration, but since we moved this to Run 3 by default, the mvaLowPT had to be explicitly modified with the Run 2 variables.
+code-checks
Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44322/39392
- This PR adds an extra 36KB to repository
Pull request #44322 was updated. @davidlange6, @vlimant, @hqucms, @antoniovilela, @fabiocos, @rappoccio, @cmsbuild can you please check and sign again.
type egamma
Hi, comments addressed. I also spotted an issue in the Era Run3 modifier: since run2_muon_2018 was not being explicitly removed there, it was loading the Run 2 weights anyway... Should I also put run2_muon_2017 in that exclusion file? I think now it should be only reading 2022 when the Era config is not run2_muon_2016, 2017 or 2018... But following all this modifiers is a little bit complicated.
+code-checks
Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44322/39405
-
This PR adds an extra 36KB to repository
-
Found files with invalid states:
- Configuration/Eras/python/Modifier_run3_muon_2023_cff.py:
- Added: ba550c090f5a34bf49ecfda0430eb485aaa8d885
- Deleted: f8ae9868255f06bf4f627677fe17b1950699d079
- Configuration/Eras/python/Modifier_run3_muon_2022_cff.py:
- Added: ba550c090f5a34bf49ecfda0430eb485aaa8d885
- Deleted: f8ae9868255f06bf4f627677fe17b1950699d079
- Configuration/Eras/python/Modifier_run3_muon_2023_cff.py:
Pull request #44322 was updated. @fabiocos, @rappoccio, @vlimant, @hqucms, @cmsbuild, @antoniovilela, @davidlange6 can you please check and sign again.
I also spotted an issue in the Era Run3 modifier: since run2_muon_2018 was not being explicitly removed there, it was loading the Run 2 weights anyway...
By quick look excluding run2_muon_2018
modifier from Run3
era would be consistent with how run2_muon201[67]
modifiers are handled in Run2_201[78]
eras. That change would seem to impact only two places.
First case is https://github.com/cms-sw/cmssw/blob/2e5dc6dfc7a5b5c7fc69b46f79859ee02b97e3b3/PhysicsTools/NanoAOD/python/triggerObjects_cff.py#L390-L395
AFAICT the prefiringweight
module is enabled only with
https://github.com/cms-sw/cmssw/blob/2e5dc6dfc7a5b5c7fc69b46f79859ee02b97e3b3/PhysicsTools/NanoAOD/python/triggerObjects_cff.py#L422-L424
and _run2_HLTconditions
is specific to the eras Run2_201[678]
, so the change would be safe.
In the second case
https://github.com/cms-sw/cmssw/blob/2e5dc6dfc7a5b5c7fc69b46f79859ee02b97e3b3/PhysicsTools/PatAlgos/python/slimming/miniAOD_tools.py#L42
the question is whether these values should apply to Run3
as well.
(on a side note, completely irrelevant to this PR, this customization https://github.com/cms-sw/cmssw/blob/2e5dc6dfc7a5b5c7fc69b46f79859ee02b97e3b3/PhysicsTools/NanoAOD/python/triggerObjects_cff.py#L372-L375 violates 6.16 in https://cms-sw.github.io/cms_coding_rules.html#6--packaging-rules-1)
Should I also put run2_muon_2017 in that exclusion file?
In what exclusion file?
Should I also put run2_muon_2017 in that exclusion file?
In what exclusion file?
I was refering to the Eras_Run3_cff.py
file where I explicitly excluded the 2018_muon config. But I think inside the 2018_muon config one removes the 2017_muon config, so I don't see need for removing it again
I also spotted an issue in the Era Run3 modifier: since run2_muon_2018 was not being explicitly removed there, it was loading the Run 2 weights anyway...
By quick look excluding
run2_muon_2018
modifier fromRun3
era would be consistent with howrun2_muon201[67]
modifiers are handled inRun2_201[78]
eras. That change would seem to impact only two places.First case is
https://github.com/cms-sw/cmssw/blob/2e5dc6dfc7a5b5c7fc69b46f79859ee02b97e3b3/PhysicsTools/NanoAOD/python/triggerObjects_cff.py#L390-L395
AFAICT the
prefiringweight
module is enabled only withhttps://github.com/cms-sw/cmssw/blob/2e5dc6dfc7a5b5c7fc69b46f79859ee02b97e3b3/PhysicsTools/NanoAOD/python/triggerObjects_cff.py#L422-L424
and
_run2_HLTconditions
is specific to the erasRun2_201[678]
, so the change would be safe.In the second case
https://github.com/cms-sw/cmssw/blob/2e5dc6dfc7a5b5c7fc69b46f79859ee02b97e3b3/PhysicsTools/PatAlgos/python/slimming/miniAOD_tools.py#L42
the question is whether these values should apply to
Run3
as well.(on a side note, completely irrelevant to this PR, this customization
https://github.com/cms-sw/cmssw/blob/2e5dc6dfc7a5b5c7fc69b46f79859ee02b97e3b3/PhysicsTools/NanoAOD/python/triggerObjects_cff.py#L372-L375
violates 6.16 in https://cms-sw.github.io/cms_coding_rules.html#6--packaging-rules-1)
I think @JanFSchulte @Fedespring might comment better on whether these things are needed for Run 3
Should I also put run2_muon_2017 in that exclusion file?
In what exclusion file?
I was refering to the
Eras_Run3_cff.py
file where I explicitly excluded the 2018_muon config. But I think inside the 2018_muon config one removes the 2017_muon config, so I don't see need for removing it again
Right. The run2_muon_2017
is already excluded in
https://github.com/cms-sw/cmssw/blob/2e5dc6dfc7a5b5c7fc69b46f79859ee02b97e3b3/Configuration/Eras/python/Era_Run2_2018_cff.py#L24
so excluding it again in something deriving from Run2_2018
would be confusing.
I also spotted an issue in the Era Run3 modifier: since run2_muon_2018 was not being explicitly removed there, it was loading the Run 2 weights anyway...
By quick look excluding
run2_muon_2018
modifier fromRun3
era would be consistent with howrun2_muon201[67]
modifiers are handled inRun2_201[78]
eras. That change would seem to impact only two places. First case is https://github.com/cms-sw/cmssw/blob/2e5dc6dfc7a5b5c7fc69b46f79859ee02b97e3b3/PhysicsTools/NanoAOD/python/triggerObjects_cff.py#L390-L395AFAICT the
prefiringweight
module is enabled only with https://github.com/cms-sw/cmssw/blob/2e5dc6dfc7a5b5c7fc69b46f79859ee02b97e3b3/PhysicsTools/NanoAOD/python/triggerObjects_cff.py#L422-L424and
_run2_HLTconditions
is specific to the erasRun2_201[678]
, so the change would be safe. In the second case https://github.com/cms-sw/cmssw/blob/2e5dc6dfc7a5b5c7fc69b46f79859ee02b97e3b3/PhysicsTools/PatAlgos/python/slimming/miniAOD_tools.py#L42the question is whether these values should apply to
Run3
as well. (on a side note, completely irrelevant to this PR, this customization https://github.com/cms-sw/cmssw/blob/2e5dc6dfc7a5b5c7fc69b46f79859ee02b97e3b3/PhysicsTools/NanoAOD/python/triggerObjects_cff.py#L372-L375violates 6.16 in https://cms-sw.github.io/cms_coding_rules.html#6--packaging-rules-1)
I think @JanFSchulte @Fedespring might comment better on whether these things are needed for Run 3
For the L1 prefiring corrections, we do not have any new weights for Run 3 (yet), those would be in the work in the L1 group. So excluding the relevant modifications from Run 3 would make sense, since the values are meaningless for Run 3 anyway. I'm not sure what the dummy values filled in that case are, though.
As for the effective areas for MiniISO, there is no retuning for Run 3, so we would need them to stay as they are for 2018 in all Run 3 Mini/Nano versions.
NB #44392
As for the effective areas for MiniISO, there is no retuning for Run 3, so we would need them to stay as they are for 2018 in all Run 3 Mini/Nano versions.
Then, given that the present state of this PR excludes the run2_muon_2018
modifier from the Run3
era, the
https://github.com/cms-sw/cmssw/blob/2e5dc6dfc7a5b5c7fc69b46f79859ee02b97e3b3/PhysicsTools/PatAlgos/python/slimming/miniAOD_tools.py#L40-L42
needs a new line
run3_muon.toModify( process.patMuons, effectiveAreaVec = [0.0566, 0.0562, 0.0363, 0.0119, 0.0064])
or you could consolidate
(run2_muon_2017 | run2_muon_2018 | run3_muon).toModify( process.patMuons, effectiveAreaVec = [0.0566, 0.0562, 0.0363, 0.0119, 0.0064])
As for the effective areas for MiniISO, there is no retuning for Run 3, so we would need them to stay as they are for 2018 in all Run 3 Mini/Nano versions.
Then, given that the present state of this PR excludes the
run2_muon_2018
modifier from theRun3
era, thehttps://github.com/cms-sw/cmssw/blob/2e5dc6dfc7a5b5c7fc69b46f79859ee02b97e3b3/PhysicsTools/PatAlgos/python/slimming/miniAOD_tools.py#L40-L42
needs a new line
run3_muon.toModify( process.patMuons, effectiveAreaVec = [0.0566, 0.0562, 0.0363, 0.0119, 0.0064])
or you could consolidate
(run2_muon_2017 | run2_muon_2018 | run3_muon).toModify( process.patMuons, effectiveAreaVec = [0.0566, 0.0562, 0.0363, 0.0119, 0.0064])
Fixed. I opted for the first option.
+code-checks
Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44322/39468
-
This PR adds an extra 28KB to repository
-
Found files with invalid states:
- Configuration/Eras/python/Modifier_run3_muon_2023_cff.py:
- Added: ba550c090f5a34bf49ecfda0430eb485aaa8d885
- Deleted: f8ae9868255f06bf4f627677fe17b1950699d079
- Configuration/Eras/python/Modifier_run3_muon_2022_cff.py:
- Added: ba550c090f5a34bf49ecfda0430eb485aaa8d885
- Deleted: f8ae9868255f06bf4f627677fe17b1950699d079
- Configuration/Eras/python/Modifier_run3_muon_2023_cff.py:
-
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/Eras/python/Era_Run3_cff.py modified in PR(s): #44368
- File PhysicsTools/NanoAOD/python/electrons_cff.py modified in PR(s): #44392
- File PhysicsTools/NanoAOD/python/muons_cff.py modified in PR(s): #44392
- File PhysicsTools/PatAlgos/python/slimming/miniAOD_tools.py modified in PR(s): #44316, #44335, #44383
Pull request #44322 was updated. @jfernan2, @antoniovilela, @mandrenguyen, @vlimant, @rappoccio, @davidlange6, @hqucms, @cmsbuild, @fabiocos can you please check and sign again.
please test
-1
Failed Tests: UnitTests RelVals RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cae026/38125/summary.html
COMMIT: bafee581a21ce3c2b0cab672d1b1f6bb838c08c5
CMSSW: CMSSW_14_1_X_2024-03-13-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/44322/38125/install.sh
to create a dev area with all the needed externals and cmssw changes.
Unit Tests
I found 2 errors in the following unit tests:
---> test TestConfigDP_7 had ERRORS ---> test TestConfigDP_8 had ERRORS
RelVals
-
136.7611
136.7611_RunJetHT2016EreMINIAOD/step2_RunJetHT2016EreMINIAOD.log
-
136.8311
136.8311_RunJetHT2017FreMINIAOD/step2_RunJetHT2017FreMINIAOD.log
-
136.88811
136.88811_RunJetHT2018DreMINIAODUL/step2_RunJetHT2018DreMINIAODUL.log
Expand to see more relval errors ...
RelVals-INPUT
-
4.6
4.6_MinimumBias2010A/step2_MinimumBias2010A.log
-
136.72411
136.72411_RunJetHT2016B_reminiaodUL/step2_RunJetHT2016B_reminiaodUL.log
-
136.72412
136.72412_RunJetHT2016B_reminiaodUL/step2_RunJetHT2016B_reminiaodUL.log
Expand to see more relval errors ...
- 136.76111
- 136.7611
- 136.7721
- 136.77211
- 136.83111
- 136.8311
- 136.88811
- 136.9
- 140.201
- 140.202
- 140.5611
- 158.01
- 159.01
- 1325.5
- 1325.51
- 1325.516
- 1325.5161
- 1325.518
- 1325.517
- 134.0
- 144.6
- 1002.0
- 13234.0
- 13434.0
- 14034.0
- 14234.0
- 2500.3
- 2500.1
- 2500.301
- 2500.2
- 2500.211
- 2500.21
- 2500.311
- 2500.31
- 2500.314
- 2500.401
- 2500.315
- 2500.316
- 2500.4
- 2500.402
- 2500.403
- 2500.0
- 2500.01