StFmsSimulatorMaker changes following reviewer comments
Overview
Following comments made by the reviewers (Kevin and Jason) a number of modifications have been made to the code. The updated code can be found on RCF at:
/star/u/tpb/fmsSoftware/reviewFmsSimulatorMaker
The above directory includes all intermediate and output files one would expect from compiling and running the code. There is also a zipped archive in the directory containing just the necessary source code and scripts. Please follow the instructions in the provided README for up-to-date, step-by-step instructions on how to run everything and what to expect at each stage. There is online class documentation at:
http://www.star.bnl.gov/~tpb/review/StFmsSimulatorMaker/
Changes
This is a summary of changes made to StFmsSimulator. Some of the changes are direct responses to their comments; others are modifications that seemed sensible on stylistic grounds.
- A Destructor has been added.
- A Copy constructor and assignment operator are not implemented, but comments have been added stating that they are not, and that the class allocates no resources on the heap. If desired, I can provide private, do-nothing implementations for both of these if copying should not be possible, but I don't know if this is considered necessary or desirable.
- CVS tags have been added to the start of the files, and an overload of GetCVS() (this was requested by StTreeMaker:INFO messages when running bfc).
- We reordered methods so Make() is first (after the constructor & destructor) in the method list of the class declaration, and in the implementation file, to aid readability.
- Doxygen documentation comments were added to the header. The class has a brief explanation of its purpose and some links. Comments were added throughout the header to document each method. Additional comments were added to the source file to give more explantion of what is being done.
- std::fill() is now briefly explained and #include <algorithm> is explicitly included.
- Enumerated types were added for supported getDetectorId() return values.
- ROOT types replace basic C types (e.g. int is replaced with Int_t).
- GetMaker("fmsDb") replaces use of the global StFmsDbMaker pointer, gStFmsDbMaker. makeFmsHit() had to become a non-const method as a result.
- g2t tables replace use of StMcEvent. While the output of StFmsSimulatorMaker is largely the same, we notice a much larger number of very low-energy (essentially zero ADC) hits when using the g2t tables instead of StMcEvent. Perhaps this is expected, but I would appreciate confirmation from someone more expert in these matters (Jason?) to confirm this.
- The constructor implementation was moved to the source file (to avoid implementation details in the header).
- Layout, variable naming and whitespace changes were made to improve consistency and clarity of style.
- We removed the ClassImp macro from the source file. My understanding is that, at least in recent ROOT releases, this is only needed for documentation generation by THtml, which I believe is not needed.
- Replaced C-style casts with static_cast.
- Added #includes that were used but not explicitly included (on a principle of "include what you use"). Remove unused #includes.
- StRoot/StFmsSimulatorMaker is the new directory layout, replacing StRoot/StSpinPool/StFmsSimulatorMaker, where the original code was located.
The code compiles without errors or warnings under DEV (as of 2014/02/24).
The maker remains in the "bare-bones" state noted by Jason. Akio intends to develop it further as he moves on to doing simulations for an FMS preshower, but says that it meets his initial needs in its current form. We therefore request that it be incorporated into StRoot as is, so that there a reviewed, approved version in CVS with just the existing FMS configuration i.e. before pre-shower additions.
- tpb's blog
- Login or register to post comments