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
Omit calibration channels and invalid data in HCAL TP emulation #19105
Omit calibration channels and invalid data in HCAL TP emulation #19105
Conversation
A new Pull Request was created by @christopheralanwest for master. It involves the following packages: SimCalorimetry/HcalTrigPrimAlgos @cmsbuild, @rekovic, @mulhearn, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@@ -228,6 +232,9 @@ void | |||
HcalTriggerPrimitiveAlgo::addSignal(const QIE11DataFrame& frame) | |||
{ | |||
HcalDetId detId(frame.id()); | |||
// prevent QIE11 calibration channels from entering TP emulation | |||
if(detId.subdet() != HcalEndcap) return; |
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.
For future-proofing, it would be better to let this function accept HcalBarrel
DetIds as well.
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.
Thanks, good idea. I've made the change.
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
@rekovic please review/sign |
I've included some more details about this PR below. Without this fix, the HCAL trigger primitive emulation crashes when run on data in which HCAL calibration channels have been read out. To reproduce a crash, use the following file from run 294745 in which the calibration channels are read out:
The exactly command is irrelevant--the point is to run the simHcalTriggerPrimitiveDigis module. These events crashed the HLT as well and so they do not show up in standard datasets. I need to use a DQM streamer file copied by hand from P5 to provoke the crash. Including this PR eliminates the crash. To see the effect of filtering bad data on data versus emulator mismatches, I've attached plots of mismatches in run 295376 before and after this PR. In this run, the low voltage for HFM07 (the yellow blocks at ieta < 29) was off. The remaining discrepancies in HF are known to be due to a firmware bug. run295376_mismatch.pdf As simulation does not include invalid data or calibration channels, this PR should have no effect on simulation or pre-2016 data. This expectation is confirmed in the comparisons with the baseline, in which the only comparison with RECO changes (in RelVal 25202.0) is a consequence of different files being used for pileup simulation (see https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_9_2_X_2017-06-06-1100+19105/20457/validateJR/logRootQA.log) for the baseline versus the baseline+PR19105. |
+1 |
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 requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar |
+1 |
This PR excludes two types of data from TP emulation: