-
Notifications
You must be signed in to change notification settings - Fork 2
Hadrons calo entry merge #4
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.
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
base: main
Are you sure you want to change the base?
Conversation
tmadlener
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this potentially be split into two separate PRs? It looks a bit like the actions are somewhat orthogonal to the HDF5 loading functionality changes.
| std::vector<unsigned long long> m_dimsOut{}; // This is a hot fix for older HDF5 versions! | ||
| //std::vector<unsigned long> m_dimsOut{}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we solve this with pre-processor checks? I suppose HDF5 exposes that somehow. Alternatively figure out the minimum version for hdf5 to go with the original version so that we can put this into the dependencies properly.
| /// begin-of-run callback | ||
| void begin(const G4Run*) override; | ||
| /// End-of-run callback | ||
| void end(const G4Run*) override; | ||
| /// begin-of-event callback | ||
| void beginEvent(const G4Event*) ; | ||
| /// End-of-event callback | ||
| void endEvent(const G4Event*) ; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to above, only need to declare those that are actually implemented
|
|
||
| option(INSTRUMENT_MODEL "Instrument the steps of the showerModel call" OFF) | ||
|
|
||
| option(RECORD_CALO_IMPACT "Record properties of particle that triggers modelShower call" OFF) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a runtime option rather than a compile time option? Looking at the code below where it is used, it looks like the simple runtime check for this is effectively free compared to the rest of work that is done.
Co-authored-by: Thomas Madlener <thomas.madlener@desy.de>
Co-authored-by: Thomas Madlener <thomas.madlener@desy.de>
Co-authored-by: Thomas Madlener <thomas.madlener@desy.de>
Yes good idea- I've split the hadron shower functionality into MR! 5. Let's come back to this once that is merged |
(Copied from Original Repo MR! 26
This merge request adds two features:
The possibility to configure models for hadronic shower simulation (including placement in ECAL + HCAL). So far testing has been performed for single particle events loaded from HDF5 - Now split into MR! 5
The option to record information about the particle which triggers fast sim at the calorimeter face (including PiD, Energy, position, direction).
This is achieved via Run/Event actions. It is configurable via ccmake, with the output being stored in a ROOT file.