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
Update of some L1T producers for L1TNtuples (legacy modules), of the L1TExtCondProducer, and of the L1MenuViewer #36839
Conversation
…cer.cc to run the emulation on MC correctly with the uGT option
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36839/28027
|
A new Pull Request was created by @elfontan (Elisa Fontanesi) for master. It involves the following packages:
@epalencia, @cmsbuild, @cecilecaillol, @rekovic can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@@ -137,7 +137,7 @@ void L1TExtCondProducer::produce(Event& iEvent, const EventSetup& iSetup) { | |||
} | |||
|
|||
bool TriggerRulePrefireVetoBit(false); | |||
if (makeTriggerRulePrefireVetoBit_) { | |||
if (iEvent.isRealData() && makeTriggerRulePrefireVetoBit_) { | |||
// code taken from Nick Smith's EventFilter/L1TRawToDigi/plugins/TriggerRulePrefireVetoFilter.cc | |||
|
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.
@elfontan , thanks for following up on these fixes. Imho, the following change should also be included in L1TExtCondProducer.cc
to avoid the seg-fault discussed in #36786 (comment) , throwing an exception instead:
// should be 16 according to TCDSRecord.h, we only care about the last 4
if (eventHistory.size() < 4) {
- edm::LogError("L1TExtCondProducer") << "Unexpectedly small L1A history from TCDSRecord";
+ throw cms::Exception("L1TExtCondProducer") << "Unexpectedly small L1A history from TCDSRecord: (size = " << eventHistory.size() << " < 4)";
}
The reason is that LogError
does not stop the routine, but after that the code assumes that the vector has at least size==4 , and this is what caused the seg-fault.
Hi @missirol, Cheers, |
Well, this would be a question for experts of this plugin. From reading the code, I would say it's not necessary to change the other LogError calls: they signal that there is something wrong/unexpected going on, but the plugin won't crash as a result (unlike for the first LogError call, which should change to an Exception); for these other LogError calls, the worst that can happen is that |
…avoid segmentation fault
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36839/28034
|
Pull request #36839 was updated. @epalencia, @cmsbuild, @cecilecaillol, @rekovic can you please check and sign again. |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-63ed00/22098/summary.html CMS StaticAnalyzer warnings: There are 1 inherits from legacy modules warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-63ed00/22098/llvm-analysis/legacy-mod-sa.txt for details. Comparison SummarySummary:
|
+l1 |
Pull request #36839 was updated. @epalencia, @cmsbuild, @cecilecaillol, @rekovic can you please check and sign again. |
@elfontan if you want to make |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36839/28100
|
Pull request #36839 was updated. @epalencia, @cmsbuild, @cecilecaillol, @rekovic can you please check and sign again. |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-63ed00/22140/summary.html Comparison SummarySummary:
|
+l1 |
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. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
This PR includes some updates related to the L1 trigger emulation:
L1UpgradeTfMuonTreeProducer, L1UpgradeTreeProducer, L1HOTreeProducer, L1EventTreeProducer.
PR validation:
Basic tests performed successfully starting from CMSSW_12_3_X_2022-01-26-1100.
From CMSSW_12_3_X_2022-01-26-1100/src: