From a9de84f88809afaaecd7b334e3ca25ab770cde6f Mon Sep 17 00:00:00 2001 From: Long Date: Sat, 19 Nov 2022 13:58:37 +0100 Subject: [PATCH 1/5] fix HcalGPU Task collection name and add protection to collection non-exist case --- DQM/HcalCommon/interface/ValueQuantity.h | 8 +-- DQM/HcalTasks/plugins/DigiTask.cc | 2 +- .../plugins/HcalGPUComparisonTask.cc | 57 ++++++++++++++++--- .../hcalgpu_dqm_sourceclient-live_cfg.py | 4 +- 4 files changed, 55 insertions(+), 16 deletions(-) diff --git a/DQM/HcalCommon/interface/ValueQuantity.h b/DQM/HcalCommon/interface/ValueQuantity.h index 61fd5a9c5beab..33127eb238d38 100644 --- a/DQM/HcalCommon/interface/ValueQuantity.h +++ b/DQM/HcalCommon/interface/ValueQuantity.h @@ -202,8 +202,8 @@ namespace hcaldqm { {fADC_256_4, -0.5}, {fEtlog2, 0.}, {fDiffRatio, -2.5}, - {fCPUenergy, 0}, - {fGPUenergy, 0}, + {fCPUenergy, -1}, + {fGPUenergy, -1}, }; const std::map max_value = { {fN, 1000}, @@ -333,8 +333,8 @@ namespace hcaldqm { {fADC_256_4, 64}, {fEtlog2, 9}, {fDiffRatio, 50}, - {fCPUenergy, 1000}, - {fGPUenergy, 1000}, + {fCPUenergy, 1005}, + {fGPUenergy, 1005}, }; class ValueQuantity : public Quantity { public: diff --git a/DQM/HcalTasks/plugins/DigiTask.cc b/DQM/HcalTasks/plugins/DigiTask.cc index 3782c185b91f3..7b9daa4e19052 100644 --- a/DQM/HcalTasks/plugins/DigiTask.cc +++ b/DQM/HcalTasks/plugins/DigiTask.cc @@ -790,7 +790,7 @@ DigiTask::DigiTask(edm::ParameterSet const& ps) lumiCache->EvtCntLS == 1) { // Reset the bin for _cCapid_BadvsFEDvsLSmod60 at the beginning of each new LS for (std::vector::const_iterator it = _vhashFEDs.begin(); it != _vhashFEDs.end(); ++it) { HcalElectronicsId eid = HcalElectronicsId(*it); - _cCapid_BadvsFEDvsLSmod60.setBinContent(eid, _currentLS % 50, 0); + _cCapid_BadvsFEDvsLSmod60.setBinContent(eid, _currentLS % 60, 0); } } diff --git a/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc b/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc index 6a57cd01b7a9c..064c438be62b8 100644 --- a/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc +++ b/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc @@ -96,10 +96,40 @@ HcalGPUComparisonTask::HcalGPUComparisonTask(edm::ParameterSet const& ps) edm::Handle chbhe_ref; edm::Handle chbhe_target; - if (!(e.getByToken(tokHBHE_ref_, chbhe_ref))) - _logger.dqmthrow("The CPU HBHERecHitCollection \"" + tagHBHE_ref_.encode() + "\" is not available"); - if (!(e.getByToken(tokHBHE_target_, chbhe_target))) - _logger.dqmthrow("The GPU HBHERecHitCollection \"" + tagHBHE_target_.encode() + "\" is not available"); + bool gotRefCollection = true, gotTargetCollection = true; + + if (!(e.getByToken(tokHBHE_ref_, chbhe_ref))) { + edm::LogWarning("HcalGPUComparisonTask") + << "The CPU HBHERecHitCollection " << tagHBHE_ref_.encode() << " is not available" << std::endl; + gotRefCollection = false; + } + if (!(e.getByToken(tokHBHE_target_, chbhe_target))) { + edm::LogWarning("HcalGPUComparisonTask") + << "The GPU HBHERecHitCollection " << tagHBHE_target_.encode() << " is not available" << std::endl; + gotTargetCollection = false; + } + + if (gotRefCollection && !gotTargetCollection) { + for (HBHERecHitCollection::const_iterator it = chbhe_ref->begin(); it != chbhe_ref->end(); ++it) { + double energy = it->energy(); + HcalDetId did = it->id(); + energyGPUvsCPU_subdet_.fill(did, energy, -0.5); + } + return; + } + if (!gotRefCollection && gotTargetCollection) { + for (HBHERecHitCollection::const_iterator it = chbhe_target->begin(); it != chbhe_target->end(); ++it) { + double energy = it->energy(); + HcalDetId did = it->id(); + energyGPUvsCPU_subdet_.fill(did, -0.5, energy); + } + return; + } + if (!gotRefCollection && !gotTargetCollection) { + edm::LogWarning("HcalGPUComparisonTask") + << "Neither CPU nor GPU RecHit Collection available, will not fill this event." << std::endl; + return; + } auto lumiCache = luminosityBlockCache(e.getLuminosityBlock().index()); _currentLS = lumiCache->currentLS; @@ -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; + ; } for (HBHERecHitCollection::const_iterator it = chbhe_target->begin(); it != chbhe_target->end(); ++it) { @@ -140,11 +171,19 @@ HcalGPUComparisonTask::HcalGPUComparisonTask(edm::ParameterSet const& ps) } mRecHitEnergy.erase(did); - } else - edm::LogError("HcalDQM|RechitTask") << "GPU Rechit id not found in CPU Rechit id collection"; + } else { + if (energy > 2.) + edm::LogError("HcalGPUComparisonTask") + << "Energetic GPU Rechit exist, but not reconstructed by CPU. DetId = " << did << std::endl; + } + } + if (!mRecHitEnergy.empty()) { + for (auto const& rhpair : mRecHitEnergy) { + if (rhpair.second > 2.) + edm::LogError("HcalGPUComparisonTask") + << "Energetic CPU Rechit exist, but not reconstructed by GPU. DetId = " << rhpair.first << std::endl; + } } - if (!mRecHitEnergy.empty()) - edm::LogError("HcalDQM|RechitTask") << "CPU Rechit id not found in GPU Rechit id collection"; } std::shared_ptr HcalGPUComparisonTask::globalBeginLuminosityBlock(edm::LuminosityBlock const& lb, diff --git a/DQM/Integration/python/clients/hcalgpu_dqm_sourceclient-live_cfg.py b/DQM/Integration/python/clients/hcalgpu_dqm_sourceclient-live_cfg.py index 0dd5cf010e661..ce4c84b1e9003 100644 --- a/DQM/Integration/python/clients/hcalgpu_dqm_sourceclient-live_cfg.py +++ b/DQM/Integration/python/clients/hcalgpu_dqm_sourceclient-live_cfg.py @@ -102,8 +102,8 @@ # New Style Modules #------------------------------------- oldsubsystem = subsystem -process.hcalGPUComparisonTask.tagHBHE_ref = "hltHbherecoFromGPU" -process.hcalGPUComparisonTask.tagHBHE_target = "hltHbherecoLegacy" +process.hcalGPUComparisonTask.tagHBHE_ref = "hltHbherecoLegacy" +process.hcalGPUComparisonTask.tagHBHE_target = "hltHbherecoFromGPU" process.hcalGPUComparisonTask.runkeyVal = runType process.hcalGPUComparisonTask.runkeyName = runTypeName From fed538893d29071dc96920cfdc9c940728b7d013 Mon Sep 17 00:00:00 2001 From: Long Date: Sun, 20 Nov 2022 15:24:50 +0100 Subject: [PATCH 2/5] simplifying code from suggestion, tested works well --- .../plugins/HcalGPUComparisonTask.cc | 54 +++++++------------ 1 file changed, 20 insertions(+), 34 deletions(-) diff --git a/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc b/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc index 064c438be62b8..8f0d500d52b5c 100644 --- a/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc +++ b/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc @@ -93,41 +93,27 @@ HcalGPUComparisonTask::HcalGPUComparisonTask(edm::ParameterSet const& ps) /* virtual */ void HcalGPUComparisonTask::_resetMonitors(hcaldqm::UpdateFreq uf) { DQTask::_resetMonitors(uf); } /* virtual */ void HcalGPUComparisonTask::_process(edm::Event const& e, edm::EventSetup const&) { - edm::Handle chbhe_ref; - edm::Handle chbhe_target; - - bool gotRefCollection = true, gotTargetCollection = true; - - if (!(e.getByToken(tokHBHE_ref_, chbhe_ref))) { - edm::LogWarning("HcalGPUComparisonTask") - << "The CPU HBHERecHitCollection " << tagHBHE_ref_.encode() << " is not available" << std::endl; - gotRefCollection = false; - } - if (!(e.getByToken(tokHBHE_target_, chbhe_target))) { - edm::LogWarning("HcalGPUComparisonTask") - << "The GPU HBHERecHitCollection " << tagHBHE_target_.encode() << " is not available" << std::endl; - gotTargetCollection = false; - } - - if (gotRefCollection && !gotTargetCollection) { - for (HBHERecHitCollection::const_iterator it = chbhe_ref->begin(); it != chbhe_ref->end(); ++it) { - double energy = it->energy(); - HcalDetId did = it->id(); - energyGPUvsCPU_subdet_.fill(did, energy, -0.5); - } - return; - } - if (!gotRefCollection && gotTargetCollection) { - for (HBHERecHitCollection::const_iterator it = chbhe_target->begin(); it != chbhe_target->end(); ++it) { - double energy = it->energy(); - HcalDetId did = it->id(); - energyGPUvsCPU_subdet_.fill(did, -0.5, energy); + 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; - } - if (!gotRefCollection && !gotTargetCollection) { - edm::LogWarning("HcalGPUComparisonTask") - << "Neither CPU nor GPU RecHit Collection available, will not fill this event." << std::endl; + return; } From 22513e47ff9b4f4c6d33d0cc18945c0c8f1139f6 Mon Sep 17 00:00:00 2001 From: Long Date: Sun, 20 Nov 2022 15:27:12 +0100 Subject: [PATCH 3/5] remove std::endl --- DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc b/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc index 8f0d500d52b5c..013d8a6bdd26d 100644 --- a/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc +++ b/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc @@ -131,7 +131,7 @@ HcalGPUComparisonTask::HcalGPUComparisonTask(edm::ParameterSet const& ps) if (mRecHitEnergy.find(did) == mRecHitEnergy.end()) mRecHitEnergy.insert(std::make_pair(did, energy)); else - edm::LogError("HcalGPUComparisonTask") << "Duplicate Rechit from the same HcalDetId" << std::endl; + edm::LogError("HcalGPUComparisonTask") << "Duplicate Rechit from the same HcalDetId"; ; } @@ -160,14 +160,14 @@ HcalGPUComparisonTask::HcalGPUComparisonTask(edm::ParameterSet const& ps) } else { if (energy > 2.) edm::LogError("HcalGPUComparisonTask") - << "Energetic GPU Rechit exist, but not reconstructed by CPU. DetId = " << did << std::endl; + << "Energetic GPU Rechit exist, but not reconstructed by CPU. DetId = " << did; } } if (!mRecHitEnergy.empty()) { for (auto const& rhpair : mRecHitEnergy) { if (rhpair.second > 2.) edm::LogError("HcalGPUComparisonTask") - << "Energetic CPU Rechit exist, but not reconstructed by GPU. DetId = " << rhpair.first << std::endl; + << "Energetic CPU Rechit exist, but not reconstructed by GPU. DetId = " << rhpair.first; } } } From a1d3341fb5fa7076fe9dd406d26b397c9138a906 Mon Sep 17 00:00:00 2001 From: Long Date: Fri, 25 Nov 2022 13:27:17 +0100 Subject: [PATCH 4/5] There is no use to fill negative values to log scaled axes, just do nothing if comparison can't be done --- DQM/HcalCommon/interface/ValueQuantity.h | 8 +++---- .../plugins/HcalGPUComparisonTask.cc | 21 +++---------------- 2 files changed, 7 insertions(+), 22 deletions(-) diff --git a/DQM/HcalCommon/interface/ValueQuantity.h b/DQM/HcalCommon/interface/ValueQuantity.h index 33127eb238d38..61fd5a9c5beab 100644 --- a/DQM/HcalCommon/interface/ValueQuantity.h +++ b/DQM/HcalCommon/interface/ValueQuantity.h @@ -202,8 +202,8 @@ namespace hcaldqm { {fADC_256_4, -0.5}, {fEtlog2, 0.}, {fDiffRatio, -2.5}, - {fCPUenergy, -1}, - {fGPUenergy, -1}, + {fCPUenergy, 0}, + {fGPUenergy, 0}, }; const std::map max_value = { {fN, 1000}, @@ -333,8 +333,8 @@ namespace hcaldqm { {fADC_256_4, 64}, {fEtlog2, 9}, {fDiffRatio, 50}, - {fCPUenergy, 1005}, - {fGPUenergy, 1005}, + {fCPUenergy, 1000}, + {fGPUenergy, 1000}, }; class ValueQuantity : public Quantity { public: diff --git a/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc b/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc index 013d8a6bdd26d..03836382acde8 100644 --- a/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc +++ b/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc @@ -96,24 +96,9 @@ HcalGPUComparisonTask::HcalGPUComparisonTask(edm::ParameterSet const& ps) 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."; - } - + if (not(chbhe_ref.isValid() or chbhe_target.isValid())) { + edm::LogWarning("HcalGPUComparisonTask") + << "Either CPU or GPU RecHit Collection is available, will not fill this event."; return; } From 6d3e945ff4333c2e99d0527a3a88c0ba5230e93f Mon Sep 17 00:00:00 2001 From: Long Date: Fri, 25 Nov 2022 13:52:51 +0100 Subject: [PATCH 5/5] logic fix --- DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc b/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc index 03836382acde8..162266f0e2119 100644 --- a/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc +++ b/DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc @@ -96,9 +96,9 @@ HcalGPUComparisonTask::HcalGPUComparisonTask(edm::ParameterSet const& ps) auto const chbhe_ref = e.getHandle(tokHBHE_ref_); auto const chbhe_target = e.getHandle(tokHBHE_target_); - if (not(chbhe_ref.isValid() or chbhe_target.isValid())) { + if (not(chbhe_ref.isValid() and chbhe_target.isValid())) { edm::LogWarning("HcalGPUComparisonTask") - << "Either CPU or GPU RecHit Collection is available, will not fill this event."; + << "Either CPU or GPU RecHit Collection is unavailable, will not fill this event."; return; }