[NGT] Add workflow to produce Phase2 NANOAOD with L1 and HLT objects#51004
[NGT] Add workflow to produce Phase2 NANOAOD with L1 and HLT objects#51004elenavernazza wants to merge 5 commits into
Conversation
|
cms-bot internal usage |
|
type ngt |
@elenavernazza can you add it to the and here: Finally can you please update the documentation at: https://github.com/cms-sw/cmssw/blob/master/Configuration/PyReleaseValidation/README.md for the new |
| '--datatier':'GEN-SIM-DIGI-RAW,NANOAODSIM', | ||
| '--procModifiers': 'ngtScouting,nano_l1_hlt', | ||
| '--eventcontent':'FEVTDEBUGHLT,NANOAODSIM' | ||
| } |
There was a problem hiding this comment.
Thinking about manual configuration, will this step run if the user forgets to specify @Phase2L1DPGwithGen, despite having nano_l1_hlt active?
There was a problem hiding this comment.
why would a user do that? Following wrong recipes results typically in wrong results, no?
There was a problem hiding this comment.
Sure; I was just suggesting that adding some protection might help.
There was a problem hiding this comment.
The NANO:@NGTScoutingVal+@Phase2L1DPGwithGen step should be the only ingredient required to produce the L1+HLT NanoAOD sample.
The nano_l1_hlt procModifier is introduced as a protection against missing PAT collections that are required in some of the L1Nano sequences. The alternative would have been to reorganize the L1Nano configuration itself; however, this would have modified the standard L1Nano format used by existing L1 workflows and users, which I wanted to avoid.
There was a problem hiding this comment.
@elenavernazza It is clear what nano_l1_hlt achieves. My comment was rather addressing future users that might use this new process modifier without fully knowing the required inputs. I don't know what happens if the user specifies only NANO:@NGTScoutingVal, but the potential crash might be cryptic. I was suggesting you could add some checks / protections that throw a meaningful error message if the user forgets to add @Phase2L1DPGwithGen.
There was a problem hiding this comment.
Please see #51004 (comment).
by this token, should you test all possible workflow combinations in the matrix ?
There was a problem hiding this comment.
I never suggested this.
I tested NANO:@NGTScoutingVal with nano_l1_hlt, since the bot does not test it, and since I thought that it could lead to a crash, due to the absence of @Phase2L1DPG.
There is no crash, so all is well.
There was a problem hiding this comment.
since the bot does not test it,
It is straightforward to see from the code changes that this should not lead to a runtime crash.
The only modifications affecting NANO:@NGTScoutingVal are in HLTrigger/NGTScouting/python/HLTNanoProducer_cff.py and are limited to:
- removing the
dstTriggerAcceptFilterfilter for the production of the HLT tables - removing the event selection in the output
As already stated in the PR description, these changes simply allow all events to be written to the output NanoAOD, independently of the L1/HLT decision. No producer dependencies or runtime execution paths are modified in a way that could plausibly introduce a crash.
Because of that, I am a bit puzzled that this specific combination of steps/process modifiers was singled out in the review, while there are already many other untested combinations present in CMSSW that would be equally (or more) likely to expose runtime issues.
| process.l1tPh2NanoTask.add(p2L1TablesTask) | ||
|
|
||
| # This modifier excludes the hpsTauTable, which is based on the l1tHPSPFTauProducerPuppi collection, no longer available in the L1 menu. | ||
| # This allows to run the NANOAOD production from L1 and HLT steps, whitout assuming old inputs from the Spring24 datasets. |
There was a problem hiding this comment.
Just a nitpick: what do you mean by "assuming" here? Maybe "considering" or "assuming the existence"? Same below.
There was a problem hiding this comment.
In case you change something else (otherwise ignore this comment): there is also a typo in "whitout".
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51004/49420
|
|
A new Pull Request was created by @elenavernazza for master. It involves the following packages:
@AdrianoDee, @BenjaminRS, @DickyChant, @Martin-Grunewald, @antoniovagnerini, @battibass, @cmsbuild, @davidlange6, @fabiocos, @ftenchini, @ftorrresd, @kfjack, @mandrenguyen, @miquork, @mmusich, @quinnanm, @sroychow can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
Pull request #51004 was updated. @AdrianoDee, @BenjaminRS, @DickyChant, @Martin-Grunewald, @antoniovagnerini, @battibass, @cmsbuild, @davidlange6, @fabiocos, @ftenchini, @ftorrresd, @kfjack, @mandrenguyen, @miquork, @mmusich, @quinnanm, @sroychow can you please check and sign again. |
|
test parameters:
|
|
@cmsbuild, please test |
Taking the liberty to tag @artlbv, just to make sure our understanding in this respect is correct. |
|
+1 Size: This PR adds an extra 72KB to repository Comparison SummarySummary:
|
Is there a way to estimate of how much this PR will affect storage needs? |
somewhat of an academical question as this workflow is not used in any production? |
|
+hlt |
PR description:
This PR introduces a new Phase-2 workflow to produce NanoAOD samples containing both L1 and HLT objects.
It combines the configurations of:
NANO:@Phase2L1DPGwithGenflavour from Phase2 L1Nano instructionsNANO:@NGTScoutingValflavour from Phase2 HLTNano instructionsWithout this PR, the L1+HLT NanoAOD sample could only be produced starting from
GEN-SIM-DIGI-RAW-MINIAODinputs from the TSG Spring24 samples, since the L1 NanoAOD assumes the presence of PAT collections in the input data.With this PR, it is possible to produce the L1+HLT NanoAOD sample directly from
GEN-SIMinputs, without the need to run all the reconstruction chain up to MINIAOD.A dedicated workflow (suffix
.774) has been introduced to monitor possible failures during future developments.The workflow is based on the procModifier
nano_l1_hlt, whichhpsTauTablesince the corresponding input collection was present in the Spring24 samples but is no longer available in more recent L1 menus (here)dstTriggerAcceptFilterfilter for the production of the HLT Tables (here) and removes the event selection in the output (here): these changes allow us to store all events in the output NanoAOD, regardless of whether they pass the L1 and HLT selections. This feature provides a change compared to theNANO:@NGTScoutingValflavour, where only the events passing the OR of the L1 seeds are stored in the output.Instructions to run the L1+HLT NanoAOD production:
GEN-SIMinput:GEN-SIM-DIGI-RAW-MINIAODinput:PR validation:
This PR has been validated by running the new workflow
34434.774:The workflow produces correctly the output NanoAOD sample, containing the L1 and HLT Tables.
By adding the option
--customise_commands='process.options.wantSummary = True', it is possible to verify that the Nano Tables are filled independently of the L1/HLT filters: in this example, 8 events out of 10 pass theDST_PFScoutingfilter (corresponding to the OR of all L1 seeds), but all 10 events are saved in the output.The other two recipes for the L1+HLT NanoAOD production based on
GEN-SIMandGEN-SIM-DIGI-RAW-MINIAODhave also been successfully validated.