Skip to content
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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Dec 2, 2022
Merged

HcalDQM: Add protection to collection non-exist case for GPUTask #40117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a9de84f
fix HcalGPU Task collection name and add protection to collection non…
Nov 19, 2022
fed5388
simplifying code from suggestion, tested works well
Nov 20, 2022
22513e4
remove std::endl
Nov 20, 2022
a1d3341
There is no use to fill negative values to log scaled axes, just do n…
Nov 25, 2022
6d3e945
logic fix
Nov 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DQM/HcalTasks/plugins/DigiTask.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint32_t>::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);
}
}

Expand Down
32 changes: 21 additions & 11 deletions DQM/HcalTasks/plugins/HcalGPUComparisonTask.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,14 @@ 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<HBHERecHitCollection> chbhe_ref;
edm::Handle<HBHERecHitCollection> chbhe_target;
auto const chbhe_ref = e.getHandle(tokHBHE_ref_);
auto const chbhe_target = e.getHandle(tokHBHE_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");
if (not(chbhe_ref.isValid() and chbhe_target.isValid())) {
edm::LogWarning("HcalGPUComparisonTask")
<< "Either CPU or GPU RecHit Collection is unavailable, will not fill this event.";
return;
}
Copy link
Contributor

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;
  }


auto lumiCache = luminosityBlockCache(e.getLuminosityBlock().index());
_currentLS = lumiCache->currentLS;
Expand All @@ -115,7 +116,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";
;
Copy link
Contributor

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.

}

for (HBHERecHitCollection::const_iterator it = chbhe_target->begin(); it != chbhe_target->end(); ++it) {
Expand All @@ -140,11 +142,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;
}
}
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;
}
}
if (!mRecHitEnergy.empty())
edm::LogError("HcalDQM|RechitTask") << "CPU Rechit id not found in GPU Rechit id collection";
}

std::shared_ptr<hcaldqm::Cache> HcalGPUComparisonTask::globalBeginLuminosityBlock(edm::LuminosityBlock const& lb,
Expand Down
4 changes: 2 additions & 2 deletions DQM/Integration/python/clients/hcalgpu_dqm_sourceclient-live_cfg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down