Open spradlin opened 1 year ago
I am using the cmake-based build in a docker container based on the one produced by main_into_hybrid/Dockerfile.base.
I am starting from the source code head of main_into_hybrid
.
Header guards are sets of preprocessor macros in header files that prevent multiple inclusion of that header during compilation of a source file. These should not be made subject to CMake configuration. They are usually the first two and last one non-trivial line in a header file and take the form
// Only comments and whitespace
#ifndef <UNIQUE_HEADER_GUARD_VARIABLE_NAME>
#define <UNIQUE_HEADER_GUARD_VARIABLE_NAME> [val]
:
<header source code>
:
#endif
The assigned value [val]
of the preprocessor header guard variable UNIQUE_HEADER_GUARD_VARIABLE_NAME
is usually optional and arbitrary. Most of the header guards in WCSim appear to have the assigned value of 1.
The following #define
macros in WCSim header files appear to define header guards:
# Format
# include/<header_filename>:#define <UNIQUE_HEADER_GUARD_VARIABLE_NAME> [val]
include/#WCSimGenerator_Radioactivity.hh#:#define WCSimGenerator_Radioactivity_hh 1
include/GdCaptureGammas.hh:#define GdCaptureGammas_hh
include/GdNeutronHPCapture.hh:#define GdNeutronHPCapture_h 1
include/GdNeutronHPCaptureFS.hh:#define GdNeutronHPCaptureFS_h 1
include/TJNuBeamFlux.hh:#define JNuBeamFlux_hh_seen
include/TNRooTrackerVtx.hh:#define NRooTrackerVtx_hh_seen
include/WCSimDarkRateMessenger.hh:#define WCSimDarkRateMessenger_h 1
include/WCSimDetectorConstruction.hh:#define WCSimDetectorConstruction_H 1
include/WCSimDetectorMessenger.hh:#define WCSimDetectorMessenger_h 1
include/WCSimEnumerations.hh:#define WCSimEnumerations_h 1
include/WCSimEventAction.hh:#define WCSimEventAction_h 1
include/WCSimGenerator_Radioactivity.hh:#define WCSimGenerator_Radioactivity_hh 1
include/WCSimLC.hh:#define WCSimLC_H 1
include/WCSimMultiPMTParameterisation.hh:#define WCSimMultiPMTParameterisation_h 1
include/WCSimPMTObject.hh:#define WCSimWCPMTObject_h 1
include/WCSimPhysicsListFactory.hh:#define WCSimPhysicsListFactory_h 1
include/WCSimPhysicsListFactoryMessenger.hh:#define WCSimPhysicsListFactoryMessenger_h 1
include/WCSimPmtInfo.hh:#define WCSim_PmtInfo
include/WCSimPrimaryGeneratorAction.hh:#define WCSimPrimaryGeneratorAction_h
include/WCSimPrimaryGeneratorMessenger.hh:#define WCSimPrimaryGeneratorMessenger_h 1
include/WCSimRandomMessenger.hh:#define WCSimRandomMessenger_h 1
include/WCSimRandomParameters.hh:#define WCSimRandomParameters_h 1
include/WCSimRootEvent.hh:#define WCSim_RootEvent
include/WCSimRootGeom.hh:#define WCSim_RootGeom
include/WCSimRootOptions.hh:#define WCSim_RootOptions
include/WCSimRootTools.hh:#define WCSimRootTools_h 1
include/WCSimRunAction.hh:#define WCSimRunAction_h 1
include/WCSimRunActionMessenger.hh:#define WCSimRunActionMessenger_h 1
include/WCSimStackingAction.hh:#define WCSimStackingAction_H 1
include/WCSimSteppingAction.hh:#define WCSimSteppingAction_h 1
include/WCSimTrackInformation.hh:#define WCSimTrackInformation_h 1
include/WCSimTrackingAction.hh:#define WCSimTrackingAction_h
include/WCSimTrackingMessenger.hh:#define WCSimTrackingMessenger_h 1
include/WCSimTrajectory.hh:#define WCSimTrajectory_h 1
include/WCSimTuningMessenger.hh:#define WCSimTuningMessenger_h 1
include/WCSimTuningParameters.hh:#define WCSimTuningParameters_h 1
include/WCSimVisManager.hh:#define WCSimVisManager_h 1
include/WCSimWCAddDarkNoise.hh:#define WCSimWCAddDarkNoise_h 1
include/WCSimWCDAQMessenger.hh:#define WCSimWCDAQMessenger_h 1
include/WCSimWCDigi.hh:#define WCSimWCDigi_h 1
include/WCSimWCDigitizer.hh:#define WCSimWCDigitizer_h 1
include/WCSimWCHit.hh:#define WCSimWCHit_h 1
include/WCSimWCPMT.hh:#define WCSimWCPMT_h 1
include/WCSimWCSD.hh:#define WCSimWCSD_h 1
include/WCSimWCTrigger.hh:#define WCSimWCTrigger_h 1
include/WCSimWLSProperties.hh:#define WCSIM_WCSIMWLSPROPERTIES_HH
include/jhfNtuple.h:#define JHFNTUPLE_H
In producing the list of header guard variables in the previous comment, i noticed that one of the header files is named
#WCSimGenerator_Radioactivity.hh#
.
Why? What is its relationship the regularly named WCSimGenerator_Radioactivity.hh
? Something is probably wrong here.
C++11 introduced strongly typed compiler constants that are declared with constexpr
. Since we are using the C++11 standard for WCSim, compiler constants that were formerly untyped #define
macros should be converted to constexpr
constants. The following #define
s appear to define such constants.
In L37-L40:
#define RNMODEL_CONC_BOTTOM 2.63 // mBq/m^{3} -> From Nakano-san et al.
#define RNMODEL_CONC_CENTER 0.1 // mBq/m^{3} -> From Nakano-san et al.
#define RNMODEL_CONC_INTERMEDIATE 0.3 // mBq/m^{3} -> Arbitrary
These are used extensively within the header file that defines them but nowhere else. It looks like much of include/RnModel_Fit_Params.hh
is are constants. Strangely, the header does not declare the variables vParam_Z
and vParam_R2
that it modifies. What is going on with this header?
include/RnModel_Fit_Params.hh
is only included in src/WCSimGenerator_Radioactivity.cc
at L204 and at L234
. It is included TWICE. Worse, both times are in the body of a function definition, WCSimGenerator_Radioactivity::SetScenario(). OK, this is an unsafe abuse the #include
mechanism.
Spin this off into its own issue and set it aside for now.
In L15-L19
#define RNMODEL_BIN_R_MAX 10 // Highest R2 layer
#define RNMODEL_BIN_Z_MIN 2 // Lowest Z layer
#define RNMODEL_BIN_Z_MAX 16 // Highest Z layer
#define RNMODEL_NBIN_R 16 // Number of R2 layers
#define RNMODEL_NBIN_Z 18 // Number of Z layers
These are used to define hard-coded array sizes in include/WCSimGenerator_Radioactivity.hh
and to control loops over those arrays in src/WCSimGenerator_Radioactivity.cc
. This is an excellent use for constexpr
.
Of all of the hard-coded constants in src/WCSimConstructMaterials.cc
, only one seems to be defined in a preprocessor macro, L1140:
#define NUMENTRIES_TY 33 // Number of bins of wavelength to be used for the Tyvek reflectivity
The use-case is interesting. The arrays that are defined using NUMENTRIES_TY
as a length would probably be better defined as STL containers, unless there are limits imposed by the GEANT4 interface. Anyway, the existing usage is excellently suited to a constexpr
.
In L18-L21
// ELJEN EJ-286
// http://www.eljentechnology.com/products/wavelength-shifting-plastics/ej-280-ej-282-ej-284-ej-286
#define nEntries_WLS_rindex 60
#define nEntries_WLS_transmittance 33
and L103
#define nEntries_abslength_eljen 574
and L240
#define nEntries_eljen_component 380
all appear to define hard-coded array sizes for arrays. Again, excellent applications of constexpr
.
The preprocessor flag G4UI_USE
is used in the top-level main source file WCSim.cc
. The lines of code optionally included by setting the flag appear on L20-L22 and L136-L144. It looks like this is intended to allow WCSim to be run with an interactive UI. This is an excellent candidate for something to be steered by CMake.
However, in its current state, this cannot work. Note that the optionally included code that would launch an interactive session is all in an if
-block, L129-L150. The condition for executing the interactive session is that there are no command line arguments. However, L47-L53 will terminate execution if the number of command line arguments is not exactly 2.
Hmm, this is another thing that should probably be spun off into its own issue.
There are a number of preprocessor flags that control the stdout verbosity of several source files. If GEANT4 has verbosity levels for G4cout, that would be a safer solution than these optionally compiled components, but i have not yet found any indication that it does. So, a compile-time system will do, i guess. The following subsections collect some observations of the preprocessor flags used for this purpose.
Used in many source files, including
src/WCSimConstructCylinder.cc
: wraps G4cout
and std::cout
callssrc/WCSimEventAction.cc
: wraps G4cout
and std::cout
callssrc/WCSimRootEvent.cc
: wraps a std::cout
call.src/WCSimWCPMT.cc
: wraps some std::cout
statements and a counter that is reported by one of those statements but never incremented(?!).src/WCSimWCTrigger.cc
: wraps some G4cout
statementssrc/WCSimWCAddDarkNoise.cc
: wraps some G4cout
statements.src/WCSimWCDigitizer.cc
: wraps some G4cout
statements.src/WCSimWCPMT.cc
: wraps some G4cout
statements.src/WCSimWCTrigger.cc
: wraps some G4cout
statements.Used only in src/WCSimDetectorConfigs.cc
to toggle the contents of a function that serves only to produce stdout!
Used only in src/WCSimEventAction.cc
to toggle digi-specific stdout statements.
Used in
src/WCSimRootEvent.cc
: wraps cout
calls.src/WCSimRootTools.cc
: wraps cout
callsUsed in src/WCSimWCAddDarkNoise.cc
to wrap some G4cout
statements.
Used in src/WCSimWCDigi.cc
to wrap some G4cout
statements.
Used in src/WCSimWCDigitizer.cc
to wrap some G4cout
statements.
Used in src/WCSimWCTrigger.cc
to wrap some G4cout
statements and some loops whose purpose is to produce stdout.
The use of multiple verbosity flags in individual source files (e.g., DEBUG, HYPER_VERBOSITY, and WCSIMWCTRIGGER_VERBOSE in src/WCSimWCTrigger.cc
indicate intent for a hieirarchy of verbosity. The use of flags with topical or localized names, like WCSIMWCTRIGGER_VERBOSE, also indicate an intent for a topical segregation of verbosity.
It makes sense to design a system that supports these intents in a consistent and less ad hoc way. An interesting problem that has probably been solved somewhere.
CMake flags can be used to steer compilation options, like extra debugging output and GEANT4 overlap checking, that are usually coded as
#ifdef
and#ifndef
blocks. This has the advantage of collecting all such options in one placeIt should also make checking for clashes and unexpected behaviors among sets of compilation options.