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

Fix Walloc-size-larger-than warnings in SimG4CMS/Forward/*NumberingScheme #41942

Closed
wants to merge 1 commit into from
Closed

Fix Walloc-size-larger-than warnings in SimG4CMS/Forward/*NumberingScheme #41942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 0 additions & 4 deletions SimG4CMS/Forward/interface/BHMNumberingScheme.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ class BHMNumberingScheme {

unsigned int getUnitID(const G4Step* aStep) const;

// Utilities to get detector levels during a step
int detectorLevel(const G4Step*) const;
void detectorLevel(const G4Step*, int&, int*, G4String*) const;

static unsigned int packIndex(int subdet, int zside, int station);
static void unpackIndex(const unsigned int& idx, int& subdet, int& zside, int& station);
};
Expand Down
4 changes: 0 additions & 4 deletions SimG4CMS/Forward/interface/BscNumberingScheme.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@ class BscNumberingScheme {

unsigned int getUnitID(const G4Step* aStep) const;

// Utilities to get detector levels during a step
int detectorLevel(const G4Step*) const;
void detectorLevel(const G4Step*, int&, int*, G4String*) const;

static unsigned int packBscIndex(int det, int zside, int station);
static void unpackBscIndex(const unsigned int& idx);
};
Expand Down
3 changes: 0 additions & 3 deletions SimG4CMS/Forward/interface/ZdcNumberingScheme.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ class ZdcNumberingScheme {
// unpacking Unit ID for Zdc (-z=1, +z=2)
static void unpackZdcIndex(const unsigned int& idx, int& subDet, int& layer, int& fiber, int& channel, int& z);

int detectorLevel(const G4Step*) const;
void detectorLevel(const G4Step*, int&, int*, G4String*) const;

private:
int verbosity;
};
Expand Down
43 changes: 10 additions & 33 deletions SimG4CMS/Forward/src/BHMNumberingScheme.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,43 +6,20 @@

BHMNumberingScheme::BHMNumberingScheme() { LogDebug("BHMSim") << " Creating BHMNumberingScheme"; }

int BHMNumberingScheme::detectorLevel(const G4Step* aStep) const {
//Find number of levels
const G4VTouchable* touch = aStep->GetPreStepPoint()->GetTouchable();
return (touch) ? ((touch->GetHistoryDepth()) + 1) : 0;
}

void BHMNumberingScheme::detectorLevel(const G4Step* aStep, int& level, int* copyno, G4String* name) const {
//Get name and copy numbers
if (level > 0) {
const G4VTouchable* touch = aStep->GetPreStepPoint()->GetTouchable();
for (int ii = 0; ii < level; ++ii) {
int i = level - ii - 1;
name[ii] = ForwardName::getName(touch->GetVolume(i)->GetName());
copyno[ii] = touch->GetReplicaNumber(i);
}
}
}

unsigned int BHMNumberingScheme::getUnitID(const G4Step* aStep) const {
unsigned intindex = 0;
int level = detectorLevel(aStep);

LogDebug("BHMSim") << "BHMNumberingScheme number of levels= " << level;
if (level > 0) {
int* copyno = new int[level];
G4String* name = new G4String[level];
detectorLevel(aStep, level, copyno, name);
//Find number of levels
const G4VTouchable* touch = aStep->GetPreStepPoint()->GetTouchable();
int level = (touch) ? ((touch->GetHistoryDepth()) + 1) : 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not think that touch can be undefined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The check was there in the original code, I can drop it if you think it's safe.


if (level > 3) {
int subdet = copyno[0];
int zside = copyno[3];
int station = copyno[1];
intindex = packIndex(subdet, zside, station);
LogDebug("BHMSim") << "BHMNumberingScheme : subdet " << subdet << " zside " << zside << " station " << station;
}
delete[] copyno;
delete[] name;
LogDebug("BHMSim") << "BHMNumberingScheme number of levels= " << level;
if (level > 3) {
Copy link
Contributor

Choose a reason for hiding this comment

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

In previous implementation if(level>3) the fix depth is used not depended on level of 'level'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what you mean. In previous implementation if level was less than or equal to 3, the arrays filled by ::detectorLevel were not used - unless the calls in that function had side-effects

int subdet = touch->GetReplicaNumber(level - 1);
int zside = touch->GetReplicaNumber(level - 4);
int station = touch->GetReplicaNumber(level - 2);
intindex = packIndex(subdet, zside, station);
LogDebug("BHMSim") << "BHMNumberingScheme : subdet " << subdet << " zside " << zside << " station " << station;
}
LogDebug("BHMSim") << "BHMNumberingScheme : UnitID 0x" << std::hex << intindex << std::dec;

Expand Down
60 changes: 21 additions & 39 deletions SimG4CMS/Forward/src/BscNumberingScheme.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,69 +13,51 @@

BscNumberingScheme::BscNumberingScheme() { LogDebug("BscSim") << " Creating BscNumberingScheme"; }

int BscNumberingScheme::detectorLevel(const G4Step* aStep) const {
//Find number of levels
const G4VTouchable* touch = aStep->GetPreStepPoint()->GetTouchable();
return (touch) ? ((touch->GetHistoryDepth()) + 1) : 0;
}

void BscNumberingScheme::detectorLevel(const G4Step* aStep, int& level, int* copyno, G4String* name) const {
//Get name and copy numbers
if (level > 0) {
const G4VTouchable* touch = aStep->GetPreStepPoint()->GetTouchable();
for (int ii = 0; ii < level; ++ii) {
int i = level - ii - 1;
name[ii] = ForwardName::getName(touch->GetVolume(i)->GetName());
copyno[ii] = touch->GetReplicaNumber(i);
}
}
}

unsigned int BscNumberingScheme::getUnitID(const G4Step* aStep) const {
unsigned int intindex = 0;
int level = detectorLevel(aStep);

//Find number of levels
const G4VTouchable* touch = aStep->GetPreStepPoint()->GetTouchable();
int level = (touch) ? ((touch->GetHistoryDepth()) + 1) : 0;

LogDebug("BscSim") << "BscNumberingScheme number of levels= " << level;

if (level > 0) {
int* copyno = new int[level];
G4String* name = new G4String[level];
detectorLevel(aStep, level, copyno, name);

int det = 0;
int zside = 0;
int station = 0;
for (int ich = 0; ich < level; ich++) {
int copyno = touch->GetReplicaNumber(level - ich - 1);
G4String name = ForwardName::getName(touch->GetVolume(level - ich - 1)->GetName());
// new and old set up configurations are possible:
if (name[ich] == "BSC1" || name[ich] == "BSC2") {
zside = copyno[ich] - 1;
} else if (name[ich] == "BSCTrap") {
if (name == "BSC1" || name == "BSC2") {
zside = copyno - 1;
} else if (name == "BSCTrap") {
det = 0;
station = 2 * (copyno[ich] - 1);
} else if (name[ich] == "BSCTubs") {
station = 2 * (copyno - 1);
} else if (name == "BSCTubs") {
det = 1;
station = copyno[ich] - 1;
} else if (name[ich] == "BSCTTop") {
station = copyno - 1;
} else if (name == "BSCTTop") {
++station;
} else if (name[ich] == "BSC2Pad") {
} else if (name == "BSC2Pad") {
det = 2;
station = copyno[ich] - 1;
station = copyno - 1;
}

LogDebug("BscSim") << "BscNumberingScheme "
<< "ich=" << ich << "copyno" << copyno[ich] << "name=" << name[ich];
<< "ich=" << ich << "copyno" << copyno << "name=" << name;
}
intindex = packBscIndex(zside, det, station);
LogDebug("BscSim") << "BscNumberingScheme : det " << det << " zside " << zside << " station " << station
<< " UnitID 0x" << std::hex << intindex << std::dec;

for (int ich = 0; ich < level; ich++)
LogDebug("BscSim") << " name = " << name[ich] << " copy = " << copyno[ich];

for (int ich = 0; ich < level; ich++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we need this loop? LogDebug may be moved inside previous loop after line 31

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do

G4String name = ForwardName::getName(touch->GetVolume(level - ich - 1)->GetName());
int copyno = touch->GetReplicaNumber(level - ich - 1);
LogDebug("BscSim") << " name = " << name << " copy = " << copyno;
}
LogDebug("BscSim") << " packed index = 0x" << std::hex << intindex << std::dec;

delete[] copyno;
delete[] name;
}

return intindex;
Expand Down
77 changes: 28 additions & 49 deletions SimG4CMS/Forward/src/ZdcNumberingScheme.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,33 +28,33 @@ void ZdcNumberingScheme::setVerbosity(const int iv) { verbosity = iv; }

unsigned int ZdcNumberingScheme::getUnitID(const G4Step* aStep) const {
uint32_t index = 0;
int level = detectorLevel(aStep);

if (level > 0) {
int* copyno = new int[level];
G4String* name = new G4String[level];

detectorLevel(aStep, level, copyno, name);
//Find number of levels
const G4VTouchable* touch = aStep->GetPreStepPoint()->GetTouchable();
int level = (touch) ? ((touch->GetHistoryDepth()) + 1) : 0;

if (level > 0) {
int zside = 0;
int channel = 0;
int fiber = 0;
int layer = 0;
HcalZDCDetId::Section section = HcalZDCDetId::Unknown;

for (int ich = 0; ich < level; ich++) {
if (name[ich] == "ZDC") {
if (copyno[ich] == 1)
int copyno = touch->GetReplicaNumber(level - ich - 1);
G4String name = ForwardName::getName(touch->GetVolume(level - ich - 1)->GetName());
if (name == "ZDC") {
if (copyno == 1)
zside = 1;
if (copyno[ich] == 2)
if (copyno == 2)
zside = -1;
} else if (name[ich] == "ZDC_EMLayer") {
} else if (name == "ZDC_EMLayer") {
section = HcalZDCDetId::EM;
#ifdef EDM_ML_DEBUG
layer = copyno[ich];
layer = copyno;
#endif
} else if (name[ich] == "ZDC_EMFiber") {
fiber = copyno[ich];
} else if (name == "ZDC_EMFiber") {
fiber = copyno;
if (fiber < 20)
channel = 1;
else if (fiber < 39)
Expand All @@ -65,13 +65,13 @@ unsigned int ZdcNumberingScheme::getUnitID(const G4Step* aStep) const {
channel = 4;
else
channel = 5;
} else if (name[ich] == "ZDC_LumLayer") {
} else if (name == "ZDC_LumLayer") {
section = HcalZDCDetId::LUM;
layer = copyno[ich];
layer = copyno;
channel = layer;
} else if (name[ich] == "ZDC_HadLayer") {
} else if (name == "ZDC_HadLayer") {
section = HcalZDCDetId::HAD;
layer = copyno[ich];
layer = copyno;
if (layer < 6)
channel = 1;
else if (layer < 12)
Expand All @@ -82,10 +82,10 @@ unsigned int ZdcNumberingScheme::getUnitID(const G4Step* aStep) const {
channel = 4;
}
#ifdef EDM_ML_DEBUG
else if (name[ich] == "ZDC_LumGas") {
else if (name == "ZDC_LumGas") {
fiber = 1;
} else if (name[ich] == "ZDC_HadFiber") {
fiber = copyno[ich];
} else if (name == "ZDC_HadFiber") {
fiber = copyno;
}
#endif
}
Expand All @@ -107,16 +107,16 @@ unsigned int ZdcNumberingScheme::getUnitID(const G4Step* aStep) const {

edm::LogVerbatim("ForwardSim") << "ZdcNumberingScheme:"
<< " getUnitID - # of levels = " << level;
for (int ich = 0; ich < level; ich++)
edm::LogVerbatim("ForwardSim") << " " << ich << ": copyno " << copyno[ich] << " name=" << name[ich]
<< " section " << section << " zside " << zside << " layer " << layer << " fiber "
<< fiber << " channel " << channel << "packedIndex =" << intindex
<< " detId raw: " << std::hex << index << std::dec;
for (int ich = 0; ich < level; ich++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This loop is also not needed, LogVerbatim may be moved after the line 45

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do

int copyno = touch->GetReplicaNumber(level - ich - 1);
G4String name = ForwardName::getName(touch->GetVolume(level - ich - 1)->GetName());

edm::LogVerbatim("ForwardSim") << " " << ich << ": copyno " << copyno << " name=" << name << " section "
<< section << " zside " << zside << " layer " << layer << " fiber " << fiber
<< " channel " << channel << "packedIndex =" << intindex
<< " detId raw: " << std::hex << index << std::dec;
}
#endif

delete[] copyno;
delete[] name;
}

return index;
Expand Down Expand Up @@ -152,24 +152,3 @@ void ZdcNumberingScheme::unpackZdcIndex(
<< " fiber " << fiber << " channel " << channel << " zside " << z;
#endif
}

int ZdcNumberingScheme::detectorLevel(const G4Step* aStep) const {
//Find number of levels
const G4VTouchable* touch = aStep->GetPreStepPoint()->GetTouchable();
int level = 0;
if (touch)
level = ((touch->GetHistoryDepth()) + 1);
return level;
}

void ZdcNumberingScheme::detectorLevel(const G4Step* aStep, int& level, int* copyno, G4String* name) const {
//Get name and copy numbers
if (level > 0) {
const G4VTouchable* touch = aStep->GetPreStepPoint()->GetTouchable();
for (int ii = 0; ii < level; ii++) {
int i = level - ii - 1;
name[ii] = ForwardName::getName(touch->GetVolume(i)->GetName());
copyno[ii] = touch->GetReplicaNumber(i);
}
}
}