-
Notifications
You must be signed in to change notification settings - Fork 13
add Calorimeter detector and output scheme #479
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
|
I agree with your dissatisfaction |
|
I think it is worth storing this as flat, since it allows some users to read it in with |
|
Thank you @ManuelHu for checking I can fix (or someone else can take over if they have time). The question now is how to flatten this array in the python wrapper without implementing ugly things. Any idea? |
|
|
The question is how do we identify the calorimeter table. at the moment it differs just for the field names. |
|
you have not only the field names, but the name of the output scheme for each table available in the output scheme |
|
You mean via IPC? |
|
yes, this info is sent via IPC: remage/src/RMGOutputManager.cc Lines 64 to 66 in 3ab5091
|
898aab6 to
35e0910
Compare
|
This should be now good, I just need to add some tests |
|
I realized that the unflattening doesn't make sense (actually creates problems) if the user requested all the detectors to be in one single table and more than a calorimeter is defined... What should we do? |
| hit->physical_volume = pv; | ||
| hit->detector_uid = det_uid; | ||
| hit->energy_deposition += step->GetTotalEnergyDeposit(); | ||
| hit->global_time = prestep->GetGlobalTime(); |
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.
I dont understand the logic here. So there is only ever 1 hit in the Calorimeter hit collection?
And this hit sums the energy, but keeps only the physical volume, uid and time of the last hit?
So what is even the use of tracking the last physical volume etc.?
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.
Also the timestamp of the last hit is completly random. It is not even guaranteed to be the last hit in time, as the time sequence does not play any role for the order in which G4 tracks particles
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.
yeah good point, we might just keep all values and store the lowest time at the end.
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.
That would also be more useful for custom outputschemes. As a customscheme would have access to the hitcollection and can then just store away any time wanted.
And i think you would need a new hit for each hit anyways if you want to differentiate between the different registered calorimeters
|
The way it is written now, shouldnt there always only be one entry per event? So after the unflattening there would always just be one entry no matter what? And if the user requested all of the calorimeters to be in a single table, that table would contain the det_uid of the last hit. On the other hand, requesting one table per calorimeter would not really make sense, as there would only be an entry for one of the calorimeters in which the last hit occured? And that entry would contain the total energy of all calorimeters? Am i misunderstanding the current implementation? |
I'm a bit unsatisfied by the fact that we still have a jagged energy structure in the output, even if I would like to store only the total energy deposited. I'm not sure we want to implement an exception for this scheme though.