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.

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

Run3-hcx373 Correct the HcalTopology class for 2 new geometry modes #46308

Merged
merged 10 commits into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions CondTools/Geometry/test/calogeometry2026writer.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import FWCore.ParameterSet.Config as cms

process = cms.Process("CaloGeometryWriter")
from Configuration.Eras.Era_Phase2C17I13M9_cff import Phase2C17I13M9
process = cms.Process("CaloGeometryWriter",Phase2C17I13M9)
process.load('CondCore.CondDB.CondDB_cfi')
process.load('Configuration.Geometry.GeometryExtended2026D41_cff')
process.load('Configuration.Geometry.GeometryExtended2026D110_cff')
process.load('Geometry.CaloEventSetup.CaloGeometry2026DBWriter_cfi')
process.load('CondTools.Geometry.HcalParametersWriter_cff')

Expand Down
8 changes: 8 additions & 0 deletions Geometry/CaloTopology/interface/HcalTopology.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ class HcalTopology : public CaloSubdetectorTopology {
HcalDetId idBack(const HcalDetId& id) const { return hcons_->idBack(id); }

private:
bool phase1() const { return ((mode_ == HcalTopologyMode::LHC) || (mode_ == HcalTopologyMode::Run3)); }
bool phase2() const { return ((mode_ == HcalTopologyMode::SLHC) || (mode_ == HcalTopologyMode::Run4)); }
/** Get the neighbors of the given cell with higher absolute ieta */
int incAIEta(const HcalDetId& id, HcalDetId neighbors[2]) const;
/** Get the neighbors of the given cell with lower absolute ieta */
Expand Down Expand Up @@ -227,14 +229,20 @@ class HcalTopology : public CaloSubdetectorTopology {
static constexpr int kHBhalf = 1296, kHEhalf = 1296, kHOhalf = 1080, kHFhalf = 864, kHThalf = 2088, kZDChalf = 11,
kCASTORhalf = 224, kCALIBhalf = 693, kHThalfPhase1 = 2520,
kHcalhalf = kHBhalf + kHEhalf + kHOhalf + kHFhalf;
static constexpr int kHBhalfPostLS2 = 4536, kHEhalfPostLS2 = 3384, kHFhalfPostLS2 = 1728;
static constexpr int kHcalhalfPostLS2 = kHBhalfPostLS2 + kHEhalfPostLS2 + kHOhalf + kHFhalfPostLS2;
static constexpr int kSizeForDenseIndexingPreLS1 = 2 * kHcalhalf;
static constexpr int kSizeForDenseIndexingPostLS2 = 2 * kHcalhalfPostLS2;
static constexpr int kHBSizePreLS1 = 2 * kHBhalf;
static constexpr int kHESizePreLS1 = 2 * kHEhalf;
static constexpr int kHOSizePreLS1 = 2 * kHOhalf;
static constexpr int kHFSizePreLS1 = 2 * kHFhalf;
static constexpr int kHTSizePreLS1 = 2 * kHThalf;
static constexpr int kHTSizePhase1 = 2 * kHThalfPhase1;
static constexpr int kCALIBSizePreLS1 = 2 * kCALIBhalf;
static constexpr int kHBSizePostLS2 = 2 * kHBhalfPostLS2;
static constexpr int kHESizePostLS2 = 2 * kHEhalfPostLS2;
static constexpr int kHFSizePostLS2 = 2 * kHFhalfPostLS2;
static constexpr int minMaxDepth_ = 4;
static constexpr unsigned int minPhi_ = 1, maxPhi_ = 72;
static constexpr unsigned int kOffCalibHB_ = 0;
Expand Down
36 changes: 12 additions & 24 deletions Geometry/CaloTopology/src/HcalTopology.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ HcalTopology::HcalTopology(const HcalDDDRecConstants* hcons, const bool mergePos
HFSize_ = kHFSizePreLS1; // ieta * iphi * depth * 2
CALIBSize_ = kCALIBSizePreLS1;
numberOfShapes_ = 87;
} else if (mode_ == HcalTopologyMode::SLHC) { // need to know more eventually
} else { // need to know more eventually
topoVersion_ = 10;
HBSize_ = nEtaHB_ * IPHI_MAX * maxDepthHB_ * 2;
HESize_ = nEtaHE_ * maxPhiHE_ * maxDepthHE_ * 2;
Expand Down Expand Up @@ -195,18 +195,14 @@ HcalTopology::HcalTopology(HcalTopologyMode::Mode mode,
HFSize_(kHFSizePreLS1),
HTSize_(kHTSizePreLS1),
CALIBSize_(kCALIBSizePreLS1),
numberOfShapes_(
((mode == HcalTopologyMode::SLHC) || (mode_ == HcalTopologyMode::Run3) || (mode_ == HcalTopologyMode::Run4))
? 500
: 87) {
numberOfShapes_((mode_ == HcalTopologyMode::LHC) ? 87 : 500) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I want to reproduce what in L82 I should write here:

Suggested change
numberOfShapes_((mode_ == HcalTopologyMode::LHC) ? 87 : 500) {
numberOfShapes_((mode_ == HcalTopologyMode::LHC) ? 87 : ((maxPhiHE_ > 72) ? 1200 : 500)) {

Could you please explain why they are different, if they need to be so? Or fix the wrong one, if they have to be the same?

if (mode_ == HcalTopologyMode::LHC) {
topoVersion_ = 0; //DL
HBSize_ = kHBSizePreLS1; // qie-per-fiber * fiber/rm * rm/rbx * rbx/barrel * barrel/hcal
HESize_ = kHESizePreLS1; // qie-per-fiber * fiber/rm * rm/rbx * rbx/endcap * endcap/hcal
HOSize_ = kHOSizePreLS1; // ieta * iphi * 2
HFSize_ = kHFSizePreLS1; // phi * eta * depth * pm
} else if ((mode_ == HcalTopologyMode::SLHC) || (mode_ == HcalTopologyMode::Run3) ||
(mode_ == HcalTopologyMode::Run4)) { // need to know more eventually
} else { // need to know more eventually
HBSize_ = maxDepthHB * 16 * IPHI_MAX * 2;
HESize_ = maxDepthHE * (29 - 16 + 1) * maxPhiHE_ * 2;
HOSize_ = 15 * IPHI_MAX * 2; // ieta * iphi * 2
Expand Down Expand Up @@ -571,8 +567,7 @@ bool HcalTopology::validRaw(const HcalDetId& id) const {

if (ok) {
if (subdet == HcalBarrel) {
if ((mode_ == HcalTopologyMode::SLHC) || (mode_ == HcalTopologyMode::H2HE) || (mode_ == HcalTopologyMode::Run3) ||
(mode_ == HcalTopologyMode::Run4)) {
if (phase2() || (mode_ == HcalTopologyMode::H2HE)) {
bsunanda marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I understand now that this is the fix: previously Run3 was erroneously counted together with the Phase2 modes.
Please, write it in the PR description, so that one can understand from there what you are fixing.

if ((aieta > lastHBRing()) || (depth > hcons_->getMaxDepth(0, aieta, iphi, zside)) ||
(depth < hcons_->getMinDepth(0, aieta, iphi, zside)))
ok = false;
Expand All @@ -581,8 +576,7 @@ bool HcalTopology::validRaw(const HcalDetId& id) const {
ok = false;
}
} else if (subdet == HcalEndcap) {
if ((mode_ == HcalTopologyMode::SLHC) || (mode_ == HcalTopologyMode::H2HE) || (mode_ == HcalTopologyMode::Run3) ||
(mode_ == HcalTopologyMode::Run4)) {
if (phase2() || (mode_ == HcalTopologyMode::H2HE)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not include Run3 now: is it correct? (Just checking)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The logic is with or without HE

if ((depth > hcons_->getMaxDepth(1, aieta, iphi, zside)) ||
(depth < hcons_->getMinDepth(1, aieta, iphi, zside)) || (aieta < firstHERing()) || (aieta > lastHERing())) {
ok = false;
Expand Down Expand Up @@ -823,8 +817,7 @@ int HcalTopology::decAIEta(const HcalDetId& id, HcalDetId neighbors[2]) const {
void HcalTopology::depthBinInformation(
HcalSubdetector subdet, int etaRing, int iphi, int zside, int& nDepthBins, int& startingBin) const {
if (subdet == HcalBarrel) {
if ((mode_ == HcalTopologyMode::SLHC) || (mode_ == HcalTopologyMode::H2HE) || (mode_ == HcalTopologyMode::Run3) ||
(mode_ == HcalTopologyMode::Run4)) {
if (phase2() || (mode_ == HcalTopologyMode::H2HE)) {
bsunanda marked this conversation as resolved.
Show resolved Hide resolved
startingBin = hcons_->getMinDepth(0, etaRing, iphi, zside);
if (etaRing == lastHBRing()) {
nDepthBins = hcons_->getDepthEta16(1, iphi, zside) - startingBin + 1;
Expand All @@ -841,8 +834,7 @@ void HcalTopology::depthBinInformation(
}
}
} else if (subdet == HcalEndcap) {
if ((mode_ == HcalTopologyMode::SLHC) || (mode_ == HcalTopologyMode::H2HE) || (mode_ == HcalTopologyMode::Run3) ||
(mode_ == HcalTopologyMode::Run4)) {
if (phase2() || (mode_ == HcalTopologyMode::H2HE)) {
if (etaRing == firstHERing()) {
startingBin = hcons_->getDepthEta16(2, iphi, zside);
} else {
Expand Down Expand Up @@ -896,17 +888,14 @@ bool HcalTopology::incrementDepth(HcalDetId& detId) const {
} else if (subdet == HcalBarrel && etaRing == lastHBRing()) {
// overlap
subdet = HcalEndcap;
if ((mode_ == HcalTopologyMode::SLHC) || (mode_ == HcalTopologyMode::H2HE) || (mode_ == HcalTopologyMode::Run3) ||
(mode_ == HcalTopologyMode::Run4))
if (phase2() || (mode_ == HcalTopologyMode::H2HE))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not include Run3 now: is it correct? (Just checking)

depth = hcons_->getDepthEta16(2, iphi, zside);
} else if ((subdet == HcalEndcap) && (etaRing == lastHERing() - 1) && (mode_ != HcalTopologyMode::SLHC) &&
(mode_ != HcalTopologyMode::Run3) && (mode_ != HcalTopologyMode::Run4)) {
} else if ((subdet == HcalEndcap) && (etaRing == lastHERing() - 1) && !phase2()) {
// guard ring HF29 is behind HE 28
subdet = HcalForward;
(ieta > 0) ? ++ieta : --ieta;
depth = 1;
} else if ((subdet == HcalEndcap) && (etaRing == lastHERing()) && (mode_ != HcalTopologyMode::SLHC) &&
(mode_ != HcalTopologyMode::Run3) && (mode_ != HcalTopologyMode::Run4)) {
} else if ((subdet == HcalEndcap) && (etaRing == lastHERing()) && !phase2()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

// split cells go to bigger granularity. Ring 29 -> 28
(ieta > 0) ? --ieta : ++ieta;
} else {
Expand Down Expand Up @@ -940,8 +929,7 @@ bool HcalTopology::decrementDepth(HcalDetId& detId) const {
}
}
} else if ((subdet == HcalEndcap) && (etaRing == lastHERing()) && (depth == hcons_->getDepthEta29(iphi, zside, 0)) &&
(mode_ != HcalTopologyMode::SLHC) && (mode_ != HcalTopologyMode::Run3) &&
(mode_ != HcalTopologyMode::Run4)) {
!phase2()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, HcalTopologyMode::Run3 in not in your definition of phase2() as in L175

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Run3 is not a part of Phase2

(ieta > 0) ? --ieta : ++ieta;
} else if (depth <= 0) {
if (subdet == HcalForward && etaRing == firstHFRing()) {
Expand Down Expand Up @@ -1143,7 +1131,7 @@ std::pair<double, double> HcalTopology::etaRange(HcalSubdetector subdet, int ket
return std::pair<double, double>(etaTableHF[ii], etaTableHF[ii + 1]);
}
} else {
int ietal = (mode_ == HcalTopologyMode::LHC && ieta == lastHERing_ - 1) ? (ieta + 1) : ieta;
int ietal = (phase1() && ieta == lastHERing_ - 1) ? (ieta + 1) : ieta;
if ((ietal < (int)(etaTable.size())) && (ieta > 0))
return std::pair<double, double>(etaTable[ieta - 1], etaTable[ietal]);
}
Expand Down