Skip to content

Commit

Permalink
fix minor things for strict compiler settings (#331)
Browse files Browse the repository at this point in the history
In the hunt for some ugly non-reproducibility bug, I used strict
compiler settings with `add_compile_options(-Wall -Wextra -Werror
-Wuninitialized)`

This PR cleans unused variables and incorrect types that don't have any
effect but prevent the usage of strict compiler settings.
  • Loading branch information
SeverinDiederichs authored Jan 10, 2025
1 parent 9697480 commit 3c9c849
Show file tree
Hide file tree
Showing 12 changed files with 31 additions and 40 deletions.
2 changes: 0 additions & 2 deletions examples/Example1/src/EventAction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ EventAction::~EventAction() {}

void EventAction::BeginOfEventAction(const G4Event *)
{
auto eventId = G4EventManager::GetEventManager()->GetConstCurrentEvent()->GetEventID();

fTimer.Start();
}

Expand Down
1 change: 0 additions & 1 deletion examples/Example1/src/RunAction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ RunAction::~RunAction() {}
void RunAction::BeginOfRunAction(const G4Run *)
{
fTimer.Start();
auto tid = G4Threading::G4GetThreadId();
}

//....oooOO0OOooo........oooOO0OOooo........oooOO0OOooo........oooOO0OOooo......
Expand Down
4 changes: 2 additions & 2 deletions examples/Example1/src/TrackingAction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ TrackingAction::TrackingAction() : G4UserTrackingAction() {}

//....oooOO0OOooo........oooOO0OOooo........oooOO0OOooo........oooOO0OOooo......

void TrackingAction::PreUserTrackingAction(const G4Track *aTrack)
void TrackingAction::PreUserTrackingAction(const G4Track *)
{
// Reset step counter
fSteppingAction->SetNumSteps(0);
}

//....oooOO0OOooo........oooOO0OOooo........oooOO0OOooo........oooOO0OOooo......

void TrackingAction::PostUserTrackingAction(const G4Track *aTrack)
void TrackingAction::PostUserTrackingAction(const G4Track *)
{
// Reset step counter
fSteppingAction->SetNumSteps(0);
Expand Down
2 changes: 0 additions & 2 deletions examples/IntegrationBenchmark/src/EventAction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ EventAction::~EventAction() {}

void EventAction::BeginOfEventAction(const G4Event *)
{
auto eventId = G4EventManager::GetEventManager()->GetConstCurrentEvent()->GetEventID();

fTimer.Start();

// Get the Run object associated to this thread and start the timer for this event
Expand Down
19 changes: 10 additions & 9 deletions examples/IntegrationBenchmark/src/Run.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@ void Run::EndOfRunSummary(G4String aOutputDirectory, G4String aOutputFilename)
double eventMean = fTestManager->getAccumulator(accumulators::EVENT_SUM) / GetNumberOfEvent();
double eventStdev = STDEV(GetNumberOfEvent(), eventMean, fTestManager->getAccumulator(accumulators::EVENT_SQ));

double nonEMMean = fTestManager->getAccumulator(accumulators::NONEM_SUM) / GetNumberOfEvent();
double nonEMStdev = STDEV(GetNumberOfEvent(), nonEMMean, fTestManager->getAccumulator(accumulators::NONEM_SQ));

double ecalMean = fTestManager->getAccumulator(accumulators::ECAL_SUM) / GetNumberOfEvent();
double ecalStdev = STDEV(GetNumberOfEvent(), ecalMean, fTestManager->getAccumulator(accumulators::ECAL_SQ));
// currently unused:
// double nonEMMean = fTestManager->getAccumulator(accumulators::NONEM_SUM) / GetNumberOfEvent();
// double nonEMStdev = STDEV(GetNumberOfEvent(), nonEMMean, fTestManager->getAccumulator(accumulators::NONEM_SQ));
// double ecalMean = fTestManager->getAccumulator(accumulators::ECAL_SUM) / GetNumberOfEvent();
// double ecalStdev = STDEV(GetNumberOfEvent(), ecalMean, fTestManager->getAccumulator(accumulators::ECAL_SQ));

G4cout << "------------------------------------------------------------"
<< "\n";
Expand All @@ -75,16 +75,17 @@ void Run::EndOfRunSummary(G4String aOutputDirectory, G4String aOutputFilename)
TestManager<std::string> aOutputTestManager;

// aBenchmarkStates->size() should correspond to the number of events
for (int i = 0; i < aBenchmarkStates->size(); i++) {
for (size_t i = 0; i < aBenchmarkStates->size(); i++) {
if (fRunAction->GetDoValidation()) {
// If we are taking validation data, export it to the specified file
// Each benchmark state contains one counter per LogicalVolume
// Export one CSV containing a list of volume IDs and Edep per event
// for (auto iter = (*aBenchmarkStates)[i].begin(); iter != (*aBenchmarkStates)[i].end(); ++iter) {
// if(iter->first >= Run::accumulators::NUM_ACCUMULATORS)
// aOutputTestManager.setAccumulator(std::to_string(iter->first - Run::accumulators::NUM_ACCUMULATORS), iter->second);
// aOutputTestManager.setAccumulator(std::to_string(iter->first - Run::accumulators::NUM_ACCUMULATORS),
// iter->second);
// }

// aOutputTestManager.setOutputDirectory(aOutputDirectory);
// aOutputTestManager.setOutputFilename(aOutputFilename);
// aOutputTestManager.exportCSV(false);
Expand Down Expand Up @@ -116,4 +117,4 @@ void Run::EndOfRunSummary(G4String aOutputDirectory, G4String aOutputFilename)
aOutputTestManager.setOutputDirectory(aOutputDirectory);
aOutputTestManager.setOutputFilename(aOutputFilename + "_global");
aOutputTestManager.exportCSV();
}
}
4 changes: 2 additions & 2 deletions examples/IntegrationBenchmark/src/TrackingAction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ TrackingAction::TrackingAction() : G4UserTrackingAction() {}

//....oooOO0OOooo........oooOO0OOooo........oooOO0OOooo........oooOO0OOooo......

void TrackingAction::PreUserTrackingAction(const G4Track *aTrack)
void TrackingAction::PreUserTrackingAction(const G4Track *)
{
// Reset step counter
fSteppingAction->SetNumSteps(0);
}

//....oooOO0OOooo........oooOO0OOooo........oooOO0OOooo........oooOO0OOooo......

void TrackingAction::PostUserTrackingAction(const G4Track *aTrack)
void TrackingAction::PostUserTrackingAction(const G4Track *)
{
// Reset step counter
fSteppingAction->SetNumSteps(0);
Expand Down
2 changes: 1 addition & 1 deletion include/AdePT/core/AdePTTransport.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class AdePTTransport : public AdePTTransportInterface {
int fMaxBatch{0}; ///< Max batch size for allocating GPU memory
int fNumVolumes{0}; ///< Total number of active logical volumes
int fNumSensitive{0}; ///< Total number of sensitive volumes
int fBufferThreshold{20}; ///< Buffer threshold for flushing AdePT transport buffer
size_t fBufferThreshold{20}; ///< Buffer threshold for flushing AdePT transport buffer
int fDebugLevel{1}; ///< Debug level
int fCUDAStackLimit{0}; ///< CUDA device stack limit
GPUstate *fGPUstate{nullptr}; ///< CUDA state placeholder
Expand Down
4 changes: 1 addition & 3 deletions include/AdePT/core/AdePTTransport.icc
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,6 @@ void AdePTTransport<IntegrationLayer>::Initialize(bool common_data)

// Do the material-cut couple index mapping once
// as well as set flags for sensitive volumes and region
// Also set the mappings from sensitive volumes to hits and VecGeom to G4 indices
int *sensitive_volumes = nullptr;

// Check VecGeom geometry matches Geant4. Initialize auxiliary per-LV data. Initialize scoring map.
fIntegrationLayer.CheckGeometry(fg4hepem_state);
Expand All @@ -181,7 +179,7 @@ void AdePTTransport<IntegrationLayer>::Initialize(bool common_data)
return;
}

fIntegrationLayer.InitScoringData(VolAuxArray::GetInstance().fAuxData);
fIntegrationLayer.InitScoringData();

std::cout << "=== AdePTTransport: initializing transport engine for thread: " << fIntegrationLayer.GetThreadID()
<< std::endl;
Expand Down
2 changes: 1 addition & 1 deletion include/AdePT/integration/AdePTGeant4Integration.hh
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public:
std::vector<std::string> const *gpuRegionNames);

/// @brief Initializes the mapping of VecGeom to G4 volumes for sensitive volumes and their parents
void InitScoringData(adeptint::VolAuxData *volAuxData);
void InitScoringData();

/// @brief Reconstructs GPU hits on host and calls the user-defined sensitive detector code
void ProcessGPUHit(GPUHit const &hit);
Expand Down
21 changes: 9 additions & 12 deletions src/AdePTGeant4Integration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ void visitGeometry(G4VPhysicalVolume const *g4_pvol, vecgeom::VPlacedVolume cons
const auto g4_lvol = g4_pvol->GetLogicalVolume();
const auto vg_lvol = vg_pvol->GetLogicalVolume();

const int nd = g4_lvol->GetNoDaughters();
const size_t nd = g4_lvol->GetNoDaughters();
const auto daughters = vg_lvol->GetDaughters();

if (nd != daughters.size()) throw std::runtime_error("Fatal: CheckGeometry: Mismatch in number of daughters");
Expand Down Expand Up @@ -158,7 +158,7 @@ void visitGeometry(G4VPhysicalVolume const *g4_pvol, vecgeom::VPlacedVolume cons
throw std::runtime_error("Fatal: CheckGeometry: Volume id larger than number of volumes");

// Now do the daughters
for (int id = 0; id < g4_lvol->GetNoDaughters(); ++id) {
for (size_t id = 0; id < g4_lvol->GetNoDaughters(); ++id) {
const auto g4pvol_d = g4_lvol->GetDaughter(id);
const auto pvol_d = vg_lvol->GetDaughters()[id];

Expand Down Expand Up @@ -224,17 +224,14 @@ void AdePTGeant4Integration::InitVolAuxData(adeptint::VolAuxData *volAuxData, G4
volAuxData[vg_lvol->id()].fGPUregion = 1;
}

// Check if the logical volume is sensitive
bool sens = false;

if (g4_lvol->GetSensitiveDetector() != nullptr) {
if (volAuxData[vg_lvol->id()].fSensIndex < 0) {
G4cout << "VecGeom: Making " << vg_lvol->GetName() << " sensitive" << G4endl;
}
volAuxData[vg_lvol->id()].fSensIndex = 1;
}
// Now do the daughters
for (int id = 0; id < g4_lvol->GetNoDaughters(); ++id) {
for (size_t id = 0; id < g4_lvol->GetNoDaughters(); ++id) {
auto g4pvol_d = g4_lvol->GetDaughter(id);
auto pvol_d = vg_lvol->GetDaughters()[id];

Expand All @@ -250,7 +247,7 @@ void AdePTGeant4Integration::InitVolAuxData(adeptint::VolAuxData *volAuxData, G4
visitGeometry(g4world, vecgeomWorld);
}

void AdePTGeant4Integration::InitScoringData(adeptint::VolAuxData *volAuxData)
void AdePTGeant4Integration::InitScoringData()
{
const G4VPhysicalVolume *g4world =
G4TransportationManager::GetTransportationManager()->GetNavigatorForTracking()->GetWorldVolume();
Expand All @@ -267,7 +264,7 @@ void AdePTGeant4Integration::InitScoringData(adeptint::VolAuxData *volAuxData)
// volume in the geometry, as a step may begin in a sensitive volume and end in a non-sensitive one
fglobal_vecgeom_to_g4_map.insert(std::pair<int, const G4VPhysicalVolume *>(vg_pvol->id(), g4_pvol));
// Now do the daughters
for (int id = 0; id < g4_lvol->GetNoDaughters(); ++id) {
for (size_t id = 0; id < g4_lvol->GetNoDaughters(); ++id) {
auto g4pvol_d = g4_lvol->GetDaughter(id);
auto pvol_d = vg_lvol->GetDaughters()[id];

Expand Down Expand Up @@ -474,10 +471,10 @@ void AdePTGeant4Integration::FillG4Step(GPUHit const *aGPUHit, G4Step *aG4Step,
aPostStepPoint->SetMomentumDirection(aPostStepPointMomentumDirection); // Real data
aPostStepPoint->SetKineticEnergy(aGPUHit->fPostStepPoint.fEKin); // Real data
// aPostStepPoint->SetVelocity(0); // Missing data
if (const auto postVolume = (*fScoringObjects->fPostG4TouchableHistoryHandle)->GetVolume();
postVolume != nullptr) { // protect against nullptr if postNavState is outside
aPostStepPoint->SetTouchableHandle(*fScoringObjects->fPostG4TouchableHistoryHandle); // Real data
aPostStepPoint->SetMaterial(postVolume->GetLogicalVolume()->GetMaterial()); // Real data
if (const auto postVolume = aPostG4TouchableHandle->GetVolume();
postVolume != nullptr) { // protect against nullptr if postNavState is outside
aPostStepPoint->SetTouchableHandle(aPostG4TouchableHandle); // Real data
aPostStepPoint->SetMaterial(postVolume->GetLogicalVolume()->GetMaterial()); // Real data
aPostStepPoint->SetMaterialCutsCouple(postVolume->GetLogicalVolume()->GetMaterialCutsCouple());
}
// aPostStepPoint->SetSensitiveDetector(nullptr); // Missing data
Expand Down
2 changes: 1 addition & 1 deletion src/AdePTTrackingManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ const vecgeom::NavigationState AdePTTrackingManager::GetVecGeomFromG4State(const
// The index of the VecGeom volume on this level (that we need to push the NavState to)
// is the same as the G4 volume. The index of the G4 volume is found by matching it against
// the daughters of the parent volume, since the G4 volume itself has no index.
for (int id = 0; id < g4Volume_parent->GetLogicalVolume()->GetNoDaughters(); ++id) {
for (size_t id = 0; id < g4Volume_parent->GetLogicalVolume()->GetNoDaughters(); ++id) {
if (g4Volume == g4Volume_parent->GetLogicalVolume()->GetDaughter(id)) {
auto daughter = current_volume->GetLogicalVolume()->GetDaughters()[id];
aNavState.Push(daughter);
Expand Down
8 changes: 4 additions & 4 deletions test/testField/testField.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,15 +123,15 @@ int *CreateMCCindex(const G4VPhysicalVolume *g4world, const vecgeom::VPlacedVolu
// - FIND vecgeom::LogicalVolume corresponding to each and every G4LogicalVolume
int nphysical = 0;

int nvolumes = vecgeom::GeoManager::Instance().GetRegisteredVolumesCount();
int *mcc_index = new int[nvolumes];
unsigned int nvolumes = vecgeom::GeoManager::Instance().GetRegisteredVolumesCount();
int *mcc_index = new int[nvolumes];
memset(mcc_index, 0, nvolumes * sizeof(int));

// recursive geometry visitor lambda matching one by one Geant4 and VecGeom logical volumes
// (we need to make sure we set the right MCC index to the right volume)
typedef std::function<void(G4LogicalVolume const *, vecgeom::LogicalVolume const *)> func_t;
func_t visitAndSetMCindex = [&](G4LogicalVolume const *g4vol, vecgeom::LogicalVolume const *vol) {
int nd = g4vol->GetNoDaughters();
size_t nd = g4vol->GetNoDaughters();
auto daughters = vol->GetDaughters();
if (nd != daughters.size()) throw std::runtime_error("Mismatch in number of daughters");
// Check the couples
Expand All @@ -150,7 +150,7 @@ int *CreateMCCindex(const G4VPhysicalVolume *g4world, const vecgeom::VPlacedVolu
nphysical++;

// Now do the daughters
for (int id = 0; id < nd; ++id) {
for (size_t id = 0; id < nd; ++id) {
auto g4pvol = g4vol->GetDaughter(id);
auto pvol = daughters[id];
// VecGeom does not strip pointers from logical volume names
Expand Down

0 comments on commit 3c9c849

Please sign in to comment.