diff --git a/LArSoftWiki/The_rules_and_guidelines.md b/LArSoftWiki/The_rules_and_guidelines.md index 9d5f123..c0885af 100644 --- a/LArSoftWiki/The_rules_and_guidelines.md +++ b/LArSoftWiki/The_rules_and_guidelines.md @@ -8,11 +8,11 @@ This is a working set of rules and guidelines. Suggestions for improvements are 1. All data formats which are a) stored in the output file, b) passed between job control modules or c) written to `histos.root` should be generalizable for N wire planes and / or M PMTs. Obviously, the number of wires N_w per plane must be generalizable. - No fixed size arrays for things that relate to planes, wires, PMTs, number of TPCs. - - No “Induction” and “Collection” data members in base classes for reconstruction or simulation. (For usability it is acceptable to have `GetCollectionPlane()` and `GetInductionPlane()` as methods of the class to return, for example, the first and last wire plane). One can test whether a plane is induction or collection using the [Geometry interface](https://code-doc.larsoft.org/doc/latest/html/classgeo_1_1GeometryCore.html) already, [`geo::PlaneGeo::SignalType()`](https://code-doc.larsoft.org/doc/latest/html/classgeo_1_1PlaneGeo.html#afee6843450e4e10af008a8dbce02d7b3), so if you have a plane number you can ask the Geometry if it is induction or collection. + - No “Induction” and “Collection” data members in base classes for reconstruction or simulation. (For usability it is acceptable to have `GetCollectionPlane()` and `GetInductionPlane()` as methods of the class to return, for example, the first and last wire plane). One can test whether a plane is induction or collection using the [Geometry interface](https://code-doc.larsoft.org/docs/latest/html/classgeo_1_1GeometryCore.html) already, [`geo::PlaneGeo::SignalType()`](https://code-doc.larsoft.org/docs/latest/html/classgeo_1_1PlaneGeo.html#afee6843450e4e10af008a8dbce02d7b3), so if you have a plane number you can ask the Geometry if it is induction or collection. - No hard-coded assumptions in data containers about positions, spacings, etc. That information should only come from the Geometry, as is stated in the Geometry package documentation. 2. An example of something that breaks from the N+M generalizability guideline is the front end electronics simulation. That will be specific to a given detector. So for that simulation it is ok, but still not great practice and certainly not to be encouraged. That is the last step of the simulation. But, in the Geant4 and all other parts of the simulation, keep it detector agnostic. 3. All reconstruction code should be written as generally as possible. Setting up the data ready to be processed should be done for N planes. Output should be given for N planes. Actually developing the algorithm to run for N planes may be more difficult – in this case, the package developer should: - - write as much of their code module as possible for N planes - always vectors, never arrays! Getting this information from the Geometry is the way to do this – i.e. one gets the number of planes from the Geometry and write your loops based on that. See code snippets in the [“Good” example](The rules and guidelines#Looping-over-geometry-entities) for how to do this. + - write as much of their code module as possible for N planes - always vectors, never arrays! Getting this information from the Geometry is the way to do this – i.e. one gets the number of planes from the Geometry and write your loops based on that. See code snippets in the [“Good” example](https://larsoft.github.io/LArSoftWiki/The_rules_and_guidelines#Looping-over-geometry-entities) for how to do this. - indicate with comments the start and end of the 2-plane specific section of code, and check for 2 planes and throw an exception if necessary. Never write code that assumes a hard coded number of planes. When in doubt of whether such code can be written, contact the conveners for guidance. We believe that using the Geometry methods such an exception to protocol should never be necessary. One unfortunate case may be inherited, legacy code whose cleanup is being deferred to the very busy original author. 4. Wire position and pitch and PMT position and orientation should be read only from the geometry file. Anywhere where a specific configuration is required should be checked for in the code with clear exceptions thrown if necessary, and detailed on the wiki. @@ -30,16 +30,16 @@ Conventions to keep in mind when writing code for LArSoft include the following: 4. Variable names should be reasonably descriptive for the scope in which they are used - `i` is ok in a small for loop, not ok in one spanning 20 lines or more. 5. Typedefs for predefined types are discouraged, i.e. `typedef int Int_t`, `typedef std::vector dubvec`. Typedefs should be reserved for legitimate new types, i.e., `Origin_t` in `SimulationBase/MCTruth.h`. 6. **Comments are mandatory** - each new object should have a description of its purpose in the header file. -7. Comments should be of a format that enables [doxygen](https://www.stack.nl/~dimitri/doxygen/docblocks.html) to interpret them. -8. Use the [message facility](https://cdcvs.fnal.gov/redmine/projects/messagefacility/wiki/Using_MessageFacility) (see also its [configuration tutorial](https://cdcvs.fnal.gov/redmine/projects/messagefacility/wiki/Tutorial_for_MessageFacility_v12_Configuration)) for output to the screen. -9. Non-module classes should use consistent header and implementation file names, e.g., al algorithm implemented in class `MyAlgo` should be written into `MyAlgo.h` and `MyAlgo.cxx`. -10. Module types and file names should be consistent. For example, a module named `MyModule` should be declared, defined, and implemented in `MyModule_module.cc`. -11. Include statements should use full paths to header files, where the full path to any LArSoft header is everything up to, but not including, the top level of the repository (e.g., `#include "larcore/Geometry/GeometryCore.h"`). -12. Only use TObject subclasses when absolutely required since it can result in considerable and unnecessary memory and CPU overheads. -13. All for and if statements should be followed by explicit braces. Failure to do so can cause trouble with maintenance. -14. Data products should be selected using art::InputTag values whenever possible. -15. Creation and filling of TTrees should be confined to a separate analyzer module that is used for debugging only. While embedded TTrees for diagnostic purposed is valuable in developing new algorithm code, these should be removed from production code. -16. Whenever possible, use constexpr instead of static const and avoid using non-const statics since they are a source of inefficiency in multi-threaded code. +7. Comments should be of a format that enables [doxygen](https://www.doxygen.nl/manual/docblocks.html) to interpret them. +9. Use the [message facility](https://cdcvs.fnal.gov/redmine/projects/messagefacility/wiki/Using_MessageFacility) (see also its [configuration tutorial](https://cdcvs.fnal.gov/redmine/projects/messagefacility/wiki/Tutorial_for_MessageFacility_v12_Configuration)) for output to the screen. +10. Non-module classes should use consistent header and implementation file names, e.g., al algorithm implemented in class `MyAlgo` should be written into `MyAlgo.h` and `MyAlgo.cxx`. +11. Module types and file names should be consistent. For example, a module named `MyModule` should be declared, defined, and implemented in `MyModule_module.cc`. +12. Include statements should use full paths to header files, where the full path to any LArSoft header is everything up to, but not including, the top level of the repository (e.g., `#include "larcore/Geometry/GeometryCore.h"`). +13. Only use TObject subclasses when absolutely required since it can result in considerable and unnecessary memory and CPU overheads. +14. All for and if statements should be followed by explicit braces. Failure to do so can cause trouble with maintenance. +15. Data products should be selected using art::InputTag values whenever possible. +16. Creation and filling of TTrees should be confined to a separate analyzer module that is used for debugging only. While embedded TTrees for diagnostic purposed is valuable in developing new algorithm code, these should be removed from production code. +17. Whenever possible, use constexpr instead of static const and avoid using non-const statics since they are a source of inefficiency in multi-threaded code. ## Uniform code format