-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[PPS] Added gtStage2DigisAlCaRecoProducer to ALCARECOPRODUCER #44044
[PPS] Added gtStage2DigisAlCaRecoProducer to ALCARECOPRODUCER #44044
Conversation
cms-bot internal usage |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44044/38985
|
A new Pull Request was created by @grzanka for master. It involves the following packages:
@cmsbuild, @consuegs, @saumyaphor4252, @perrotta can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild please test |
Shouldn't these workflows be included in the tests? |
@cmsbuild please abort |
test parameters:
|
@cmsbuild please test |
|
||
ctppsDiamondRawToDigiAlCaRecoProducer = _ctppsDiamondRawToDigi.clone(rawDataTag = 'hltPPSCalibrationRaw') | ||
totemTimingRawToDigiAlCaRecoProducer = _totemTimingRawToDigi.clone(rawDataTag = 'hltPPSCalibrationRaw') | ||
ctppsPixelDigisAlCaRecoProducer = _ctppsPixelDigis.clone(inputLabel = 'hltPPSCalibrationRaw') | ||
hltGtStage2Digis = _gtStage2Digis.clone(InputLabel="hltFEDSelectorL1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mh.. the way I understand it, we try [*] to 'reserve' the name hlt*
for products that come directly from the HLT process, so here I would suggest to use a different name for hltGtStage2Digis
. Tagging @Martin-Grunewald.
[*] I think we do use hlt*
modules in some offline workflows related to RawPrime, but that's maybe a special case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @missirol please change the label to avoid hlt - maybe alcaGtStage2Digis
- and propagate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe
alcaGtStage2Digis
- and propagate.
I'd just call them gtStage2Digis
as everywhere else in offline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could change the code to :
from EventFilter.L1TRawToDigi.gtStage2Digis_cfi import gtStage2Digis as _gtStage2Digis
(...)
gtStage2Digis = _gtStage2Digis.clone(InputLabel="hltFEDSelectorL1")
but wouldn't that introduce some clash ?
With other items I was adding AlCaRecoProducer
suffix, like:
from EventFilter.CTPPSRawToDigi.ctppsRawToDigi_cff import ctppsPixelDigis as _ctppsPixelDigis
(...)
ctppsPixelDigisAlCaRecoProducer = _ctppsPixelDigis.clone(inputLabel = 'hltPPSCalibrationRaw')
Whout do you think then of using:
from EventFilter.L1TRawToDigi.gtStage2Digis_cfi import gtStage2Digis as _gtStage2Digis
(...)
gtStage2DigisAlCaRecoProducer = _gtStage2Digis.clone(InputLabel="hltFEDSelectorL1")
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whout do you think then of using
Seems unnecessarily complicated to me, there are no other producers of this collection on that kind of input data, I am not sure what clashes it could cause.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe
alcaGtStage2Digis
- and propagate.I'd just call them
gtStage2Digis
as everywhere else in offline.
After modification in my local CMSSW workspace on lxplus I get following error:
[grzankal@lxplus990 src]$ git diff
diff --git a/Calibration/PPSAlCaRecoProducer/python/ALCARECOPPSCalMaxTracks_cff.py b/Calibration/PPSAlCaRecoProducer/python/ALCARECOPPSCalMaxTracks_cff.py
index 1349baa295f..b68c5409f2f 100644
--- a/Calibration/PPSAlCaRecoProducer/python/ALCARECOPPSCalMaxTracks_cff.py
+++ b/Calibration/PPSAlCaRecoProducer/python/ALCARECOPPSCalMaxTracks_cff.py
@@ -18,13 +18,13 @@ from EventFilter.L1TRawToDigi.gtStage2Digis_cfi import gtStage2Digis as _gtStage
ctppsDiamondRawToDigiAlCaRecoProducer = _ctppsDiamondRawToDigi.clone(rawDataTag = 'hltPPSCalibrationRaw')
totemTimingRawToDigiAlCaRecoProducer = _totemTimingRawToDigi.clone(rawDataTag = 'hltPPSCalibrationRaw')
ctppsPixelDigisAlCaRecoProducer = _ctppsPixelDigis.clone(inputLabel = 'hltPPSCalibrationRaw')
-hltGtStage2Digis = _gtStage2Digis.clone(InputLabel="hltFEDSelectorL1")
+gtStage2Digis = _gtStage2Digis.clone(InputLabel="hltFEDSelectorL1")
ctppsRawToDigiTaskAlCaRecoProducer = cms.Task(
ctppsDiamondRawToDigiAlCaRecoProducer,
totemTimingRawToDigiAlCaRecoProducer,
ctppsPixelDigisAlCaRecoProducer,
- hltGtStage2Digis
+ gtStage2Digis
)
ALCARECOPPSCalMaxTracksRaw2Digi = cms.Sequence(ctppsRawToDigiTaskAlCaRecoProducer)
@@ -105,4 +105,4 @@ recoPPSSequenceAlCaRecoProducer = cms.Sequence(recoPPSTaskAlCaRecoProducer)
# 6. master sequence object
#------------------------------------------------------
-seqALCARECOPPSCalMaxTracksReco = cms.Sequence( ALCARECOPPSCalMaxTracksFilter + ALCARECOPPSCalMaxTracksRaw2Digi + recoPPSSequenceAlCaRecoProducer)
\ No newline at end of file
+seqALCARECOPPSCalMaxTracksReco = cms.Sequence( ALCARECOPPSCalMaxTracksFilter + ALCARECOPPSCalMaxTracksRaw2Digi + recoPPSSequenceAlCaRecoProducer)
[grzankal@lxplus990 src]$ cat 1044.0_RunRawPPS2022B/step2_RunRawPPS2022B.log
RAW2DIGI,L1Reco,ALCAPRODUCER:PPSCalMaxTracks,ENDJOB
entry filelist:step1_dasquery.log
found files: ['/store/data/Run2022B/AlCaPPS/RAW/v1/000/355/207/00000/c23440f4-49c0-44aa-b8f6-f40598fb4705.root']
Step: RAW2DIGI Spec:
Step: L1Reco Spec:
Step: ALCAPRODUCER Spec: ['PPSCalMaxTracks']
Traceback (most recent call last):
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/bin/el9_amd64_gcc12/cmsDriver.py", line 40, in <module>
run()
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/bin/el9_amd64_gcc12/cmsDriver.py", line 16, in run
configBuilder.prepare()
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/Configuration/Applications/python/ConfigBuilder.py", line 2310, in prepare
self.addStandardSequences()
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/Configuration/Applications/python/ConfigBuilder.py", line 850, in addStandardSequences
getattr(self,"prepare_"+stepName)(stepSpec = '+'.join(stepSpec))
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/Configuration/Applications/python/ConfigBuilder.py", line 1336, in prepare_ALCAPRODUCER
self.prepare_ALCA(stepSpec, workflow = "producers")
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/Configuration/Applications/python/ConfigBuilder.py", line 1343, in prepare_ALCA
alcaConfig,sequence,_=self.loadDefaultOrSpecifiedCFF(stepSpec,self.ALCADefaultCFF)
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/Configuration/Applications/python/ConfigBuilder.py", line 1299, in loadDefaultOrSpecifiedCFF
l=self.loadAndRemember(_cff)
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/Configuration/Applications/python/ConfigBuilder.py", line 376, in loadAndRemember
self.process.load(includeFile)
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/FWCore/ParameterSet/python/Config.py", line 762, in load
self.extend(sys.modules[moduleName])
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/FWCore/ParameterSet/python/Config.py", line 785, in extend
self.__setattr__(name,item)
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/FWCore/ParameterSet/python/Config.py", line 495, in __setattr__
raise ValueError(msg1+s.label_()+msg2)
ValueError: Trying to override definition of gtStage2Digis while it is used by the task L1TRawToDigiTask
new object defined in: /afs/cern.ch/user/g/grzankal/CMSSW_14_1_0_pre0/src/Calibration/PPSAlCaRecoProducer/python/ALCARECOPPSCalMaxTracks_cff.py
existing object defined in: /cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/EventFilter/L1TRawToDigi/python/gtStage2Digis_cfi.py
[grzankal@lxplus990 src]$
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After modification in my local CMSSW workspace on lxplus I get following error:
You should not clone it, but define it directly anyway no strong push (at least from me).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After modification in my local CMSSW workspace on lxplus I get following error:
You should not clone it, but define it directly anyway no strong push (at least from me).
Naively defining it directly I also get a clash:
[grzankal@lxplus990 src]$ git diff
diff --git a/Calibration/PPSAlCaRecoProducer/python/ALCARECOPPSCalMaxTracks_cff.py b/Calibration/PPSAlCaRecoProducer/python/ALCARECOPPSCalMaxTracks_cff.py
index 1349baa295f..e9b0bc3e1fd 100644
--- a/Calibration/PPSAlCaRecoProducer/python/ALCARECOPPSCalMaxTracks_cff.py
+++ b/Calibration/PPSAlCaRecoProducer/python/ALCARECOPPSCalMaxTracks_cff.py
@@ -13,18 +13,25 @@ from EventFilter.CTPPSRawToDigi.ctppsRawToDigi_cff import *
from EventFilter.CTPPSRawToDigi.ctppsRawToDigi_cff import ctppsDiamondRawToDigi as _ctppsDiamondRawToDigi
from EventFilter.CTPPSRawToDigi.ctppsRawToDigi_cff import totemTimingRawToDigi as _totemTimingRawToDigi
from EventFilter.CTPPSRawToDigi.ctppsRawToDigi_cff import ctppsPixelDigis as _ctppsPixelDigis
-from EventFilter.L1TRawToDigi.gtStage2Digis_cfi import gtStage2Digis as _gtStage2Digis
+#from EventFilter.L1TRawToDigi.gtStage2Digis_cfi import gtStage2Digis as _gtStage2Digis
ctppsDiamondRawToDigiAlCaRecoProducer = _ctppsDiamondRawToDigi.clone(rawDataTag = 'hltPPSCalibrationRaw')
totemTimingRawToDigiAlCaRecoProducer = _totemTimingRawToDigi.clone(rawDataTag = 'hltPPSCalibrationRaw')
ctppsPixelDigisAlCaRecoProducer = _ctppsPixelDigis.clone(inputLabel = 'hltPPSCalibrationRaw')
-hltGtStage2Digis = _gtStage2Digis.clone(InputLabel="hltFEDSelectorL1")
+#gtStage2Digis = _gtStage2Digis.clone(InputLabel="hltFEDSelectorL1")
+
+gtStage2Digis = cms.EDProducer(
+ "L1TRawToDigi",
+ InputLabel = cms.InputTag("hltFEDSelectorL1"),
+ Setup = cms.string("stage2::GTSetup"),
+ FedIds = cms.vint32( 1404 ),
+)
ctppsRawToDigiTaskAlCaRecoProducer = cms.Task(
ctppsDiamondRawToDigiAlCaRecoProducer,
totemTimingRawToDigiAlCaRecoProducer,
ctppsPixelDigisAlCaRecoProducer,
- hltGtStage2Digis
+ gtStage2Digis
)
ALCARECOPPSCalMaxTracksRaw2Digi = cms.Sequence(ctppsRawToDigiTaskAlCaRecoProducer)
@@ -105,4 +112,4 @@ recoPPSSequenceAlCaRecoProducer = cms.Sequence(recoPPSTaskAlCaRecoProducer)
# 6. master sequence object
#------------------------------------------------------
-seqALCARECOPPSCalMaxTracksReco = cms.Sequence( ALCARECOPPSCalMaxTracksFilter + ALCARECOPPSCalMaxTracksRaw2Digi + recoPPSSequenceAlCaRecoProducer)
\ No newline at end of file
+seqALCARECOPPSCalMaxTracksReco = cms.Sequence( ALCARECOPPSCalMaxTracksFilter + ALCARECOPPSCalMaxTracksRaw2Digi + recoPPSSequenceAlCaRecoProducer)
[grzankal@lxplus990 src]$ cat 1044.0_RunRawPPS2022B/step2_RunRawPPS2022B.log
RAW2DIGI,L1Reco,ALCAPRODUCER:PPSCalMaxTracks,ENDJOB
entry filelist:step1_dasquery.log
found files: ['/store/data/Run2022B/AlCaPPS/RAW/v1/000/355/207/00000/c23440f4-49c0-44aa-b8f6-f40598fb4705.root']
Step: RAW2DIGI Spec:
Step: L1Reco Spec:
Step: ALCAPRODUCER Spec: ['PPSCalMaxTracks']
Traceback (most recent call last):
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/bin/el9_amd64_gcc12/cmsDriver.py", line 40, in <module>
run()
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/bin/el9_amd64_gcc12/cmsDriver.py", line 16, in run
configBuilder.prepare()
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/Configuration/Applications/python/ConfigBuilder.py", line 2310, in prepare
self.addStandardSequences()
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/Configuration/Applications/python/ConfigBuilder.py", line 850, in addStandardSequences
getattr(self,"prepare_"+stepName)(stepSpec = '+'.join(stepSpec))
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/Configuration/Applications/python/ConfigBuilder.py", line 1336, in prepare_ALCAPRODUCER
self.prepare_ALCA(stepSpec, workflow = "producers")
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/Configuration/Applications/python/ConfigBuilder.py", line 1343, in prepare_ALCA
alcaConfig,sequence,_=self.loadDefaultOrSpecifiedCFF(stepSpec,self.ALCADefaultCFF)
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/Configuration/Applications/python/ConfigBuilder.py", line 1299, in loadDefaultOrSpecifiedCFF
l=self.loadAndRemember(_cff)
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/Configuration/Applications/python/ConfigBuilder.py", line 376, in loadAndRemember
self.process.load(includeFile)
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/FWCore/ParameterSet/python/Config.py", line 762, in load
self.extend(sys.modules[moduleName])
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/FWCore/ParameterSet/python/Config.py", line 785, in extend
self.__setattr__(name,item)
File "/cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/FWCore/ParameterSet/python/Config.py", line 495, in __setattr__
raise ValueError(msg1+s.label_()+msg2)
ValueError: Trying to override definition of gtStage2Digis while it is used by the task L1TRawToDigiTask
new object defined in: /afs/cern.ch/user/g/grzankal/CMSSW_14_1_0_pre0/src/Calibration/PPSAlCaRecoProducer/python/ALCARECOPPSCalMaxTracks_cff.py
existing object defined in: /cvmfs/cms.cern.ch/el9_amd64_gcc12/cms/cmssw/CMSSW_14_1_0_pre0/src/EventFilter/L1TRawToDigi/python/gtStage2Digis_cfi.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naively defining it directly I also get a clash
Ok, it is because the relval runs the RAW2DIGI
together with ALCA
in the same step. One could argue that only a specific raw2digi flavour is actually needed (you don't need to unpack all the FEDs in this scenario), but OK, making it cleaner involves more involved changes. Feel free to call it as you prefer after cloning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've renamed hltGtStage2Digis
to gtStage2DigisAlCaRecoProducer
in 671fafa
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dbcd41/37607/summary.html Comparison SummarySummary:
|
It seems that checks have passed. As expected we see following messages in the logfile of PPS 1044 relval: Is that tolerable ? Similar message will be printed if this PR is merged and we would re-reco 2023 (or older) data with ALCAPPSRECO producer enabled.
|
Also - to improve testing of this PR we may use the files mentioned in the PR description ( |
Polluting the logs is in general frowned upon. |
We are open to implement any reasonable switch. As for the time being I will adjust the naming as suggested above. I can also add some tests for new data scenario, but for that purpose I'd need to get some files copied into |
type ctpps |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dbcd41/37701/summary.html Comparison SummarySummary:
|
+alca
|
It seems that this PR passed the test and got first signatures. Is there anything else we need to do to get it merged ? |
it doesn't depend on me :D Allow me to ping @cms-sw/pdmv-l2 and @cms-sw/upgrade-l2 (the update in their signature area is marginal, I don't foresee issues). |
+Upgrade |
@cms-sw/pdmv-l2 Ping |
+pdmv |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @sextonkennedy, @antoniovilela, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
The PR adds the hltGtStage2Digis production step in the PPS ALCARECO sequence and preserves this product in the output. This change follows the discussion in CMSHLT-2989, in which the the hltFEDSelectorL1 products are added to the HLT output in order to have access to the prescale values in the final dataset.
A backport to CMSSW_14_0_X will be created as soon as this PR is fully checked out.
PR validation:
The PR was tested using privately-produced input files (check recipe below) and unit tests run locally. These input files (one for the express stream and one for the prompt) are available as:
The tests were executed with:
cmsDriver.py testExpressPPSAlCaRecoProducer -s ALCAPRODUCER:PPSCalMaxTracks,ENDJOB --process ALCARECO --scenario pp --era run3_common --conditions auto:run3_data_express --data --datatier ALCARECO --eventcontent ALCARECO --nThreads 8 --number 100 --filein ${INPUTFILE} --fileout file:outputALCAPPS_RECO_express.root --customise_commands="$customise_commands"
and
cmsDriver.py testPromptPPSAlCaRecoProducer -s ALCAPRODUCER:PPSCalMaxTracks,ENDJOB --process ALCARECO --scenario pp --era run3_common --conditions auto:run3_data_express --data --datatier ALCARECO --eventcontent ALCARECO --nThreads 8 --number 100 --filein ${INPUTFILE} --fileout file:outputALCAPPS_RECO_prompt.root --customise_commands="$customise_commands"
where ${INPUTFILE} is the respective file above and $customise_commands is taken from the express test-script or prompt test-script, respectively.
These input files should be moved to the /store/group/alca_global/pps_alcareco_producer_tests area and become the new test files.
Backward compatibility
The
hltGtStage2Digis
expects a product (underhltFEDSelectorL1
label) which is not present in 2023 (but will be available in 2024 data) and older datasets.We have a couple of PPS relvals (1043,1044,1045) using PPS ALCARECO producer on 2022 raw data, see PPS ALCARECO relvals.
These relvals should be used in testing of this PR, as we believe they are not part of usual set of
runTheMatrix
tests.When we tested this PR we discovered that these relvals (1043,1044,1045) were succeeding, but the step2 log files contained an expected error message (as the product was not found in 2022 raw data):
We are open for discussion on proper back-ward compatibility mode.
HLT test file production recipe
HLT test files were produced for a ZeroBias RAW file from run 367104.