-
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
HcalDQM: Add protection to collection non-exist case for GPUTask #40117
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40117/33102
|
A new Pull Request was created by @lwang046 for master. It involves the following packages:
@emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @syuvivida, @pmandrik, @micsucmed, @rvenditti can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
edm::LogWarning("HcalGPUComparisonTask") | ||
<< "Neither CPU nor GPU RecHit Collection available, will not fill this event." << std::endl; | ||
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.
Let me suggest an optional change to make this more compact; I did not test this, so please double-check this if you want to use it.
auto const chbhe_ref = e.getHandle(tokHBHE_ref_);
auto const chbhe_target = e.getHandle(tokHBHE_target_);
if (not (chbhe_ref.isValid() and chbhe_target.isValid())) {
if (chbhe_target.isValid()) {
edm::LogWarning("HcalGPUComparisonTask")
<< "The CPU HBHERecHitCollection " << tagHBHE_ref_.encode() << " is not available";
for (auto const& rh : *chbhe_target)
energyGPUvsCPU_subdet_.fill(rh.id(), -0.5, rh.energy());
}
else if (chbhe_ref.isValid()) {
edm::LogWarning("HcalGPUComparisonTask")
<< "The GPU HBHERecHitCollection " << tagHBHE_target_.encode() << " is not available";
for (auto const& rh : *chbhe_ref)
energyGPUvsCPU_subdet_.fill(rh.id(), rh.energy(), -0.5);
}
else {
edm::LogWarning("HcalGPUComparisonTask")
<< "Neither CPU nor GPU RecHit Collection available, will not fill this event.";
}
return;
}
@@ -115,7 +145,8 @@ HcalGPUComparisonTask::HcalGPUComparisonTask(edm::ParameterSet const& ps) | |||
if (mRecHitEnergy.find(did) == mRecHitEnergy.end()) | |||
mRecHitEnergy.insert(std::make_pair(did, energy)); | |||
else | |||
edm::LogError("HcalDQM|RechitTask") << "Duplicate Rechit from the same HcalDetId"; | |||
edm::LogError("HcalGPUComparisonTask") << "Duplicate Rechit from the same HcalDetId" << std::endl; | |||
; |
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.
std::endl
here is not needed, as Log*
calls ignore it and apply a newline regardless.
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40117/33107
|
Pull request #40117 was updated. @emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @syuvivida, @pmandrik, @micsucmed, @rvenditti can you please check and sign again. |
type hcal,bugfix |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-83a1a4/29162/summary.html Comparison SummarySummary:
|
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, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
apologies commented in the wrong PR |
+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 will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
HcalDQM: Add protection to collection non-exist case for GPUTask, backport of PR #40117
HcalDQM: Add protection to collection non-exist case for GPUTask, backport of PR #40117
HcalDQM: Add protection to collection non-exist case for GPUTask, backport of PR #40117
PR description:
Regarding issue #40115, this PR corrects the reference&target name mixing in the hcalgpu task, adds some conditions to the collections non-exist case instead of throwing out fetal exception, and correct a minor numeric error in the DigiTask that should mod the LS by 60, instead of 50.
PR validation:
Tested with
cmsRun DQM/Integration/python/clients/hcalgpu_dqm_sourceclient-live_cfg.py runInputDir=/afs/cern.ch/work/l/lowang/DQM_Dev/CMSSW_12_6_X_2022-11-17-2300/src/ runNumber=362217 runkey=hi_run
and shows no problems.