-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add Particle filter stacking action option #244
base: main
Are you sure you want to change the base?
Conversation
@EricMEsch one thing I don't understand is why this would be a separate output scheme? |
|
||
|
||
std::optional<G4ClassificationOfNewTrack> RMGParticleFilterOutputScheme:: | ||
StackingActionClassify(const G4Track* aTrack, int stage) { |
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.
Do we want to only stack particles after stage 1, eg. I am thinking about stacking nuclei only after they decay? Probably for nuclei its a bit difficult since you might have additional decays.
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 am fairly certain the stages are something that is inherently handled by the user (us) and not G4? So far we run one stage, meaning everything until the Particle-Stack is empty. Then we check if any OutputScheme requests a new stage. If one OutputScheme requests a new stage, all tracks that have been marked with fWaiting
will be put on the stack again (which right now can only happen to OpticalPhotons
). Otherwise they will be discarded. So i am not sure what exactly you refer to?
I guess a simple test would be to run this with eg. alphas for some alpha decay and check that all the events are removed, and also a beta decay (checking nothing changes)? |
I mean it does not have to be. It is only utilizing the hook into the StackingAction for OutputSchemes. This could also be part of the StackingAction. I just thought it would be a better way of organizing to use this structure. If we are going to add multiple things to the StackingAction later on, we would be happy if they were not all hard coded into it. Also, this means that in case the OptionalOutputScheme is not activated by macro, this code will never be executed, which is a little bit faster i would assume.
Could be worth a try, although i am not sure how Geant4 will react if you immediately kill the primary generated particle from the stack. In theory it should be fine i guess. |
please also add to Lines 48 to 54 in 8ee3d58
also, please rebase on main (on include #252) and run |
@EricMEsch I wonder if it is possible to make this volume dependent or maybe G4Region dependent. |
It should certainly both be possible. For the volume dependence i guess a user would just have to specify the name of the physical volume. I think a reasonable way to implement is:
The important thing to remember is, that this deletes on creation, so there is no way of knowing if the particle might have reached another volume. The G4Region thing should certainly also be possible, but i have not looked into G4Region yet (but i dont think its complicated). The question is do we even want both? I think specifying the volume would be totally enough |
What about having either volumes to keep or volumes to kill ? I think the most common use case is kill everywhere apart from in a sensitive detector? |
Should also be possible The logic would then be
I guess i could add a warning if a volume to keep is specified in the same simulation with a volume to kill (as those are conflicting requests) |
Yeah alternatively we just have an option for the default and sensitive regions, these are already defined and I think then you can easily check which region a volume belongs to ? ie. G4Region::BelongsTo I am not sure using the Regions is the right approach since its less easy to modify though.. |
I personally would prefer the volume by name option, but maybe that is just because i have not looked at the regions yet. |
@tdixon97 The option to add kill/keep volumes is now implemented. I am not 100% sure if the information about the physical volume actually exists at this point (as we catch the track before the tracking started), but i am pretty sure it does and we will quickly find out in testing. The only "issue" now is that you can not specify a volume per particle. You specify volumes and particles and the filter is applied to all specified particles in all specified volumes. It would be possible to use |
I think it sounds good, before doing very complicated stuff we should consider what typical use cases are, probably killing beta or alpha anywhere not germanium.
I think if people want something very fancy they will prefer custom production cuts?
…________________________________
From: Eric Esch ***@***.***>
Sent: Saturday, February 8, 2025 4:25 PM
To: legend-exp/remage ***@***.***>
Cc: Dixon, Toby ***@***.***>; Mention ***@***.***>
Subject: Re: [legend-exp/remage] Add Particle filter stacking action option (PR #244)
⚠ Caution: External sender
@tdixon97<https://github.com/tdixon97> The option to add kill/keep volumes is now implemented. I am not 100% sure if the information about the physical volume actually exists at this point (as we catch the track before the tracking started), but i am pretty sure it does and we will quickly find out in testing.
The only "issue" now is that you can not specify a volume per particle. You specify volumes and particles and the filter is applied to all specified particles in all specified volumes.
It would be possible to use std::map<int, std::vector<std::string>> instead, but this would probably mean that i would have to add a custom messenger to allow the user to specify pdg codes togheter with a volume name.
—
Reply to this email directly, view it on GitHub<#244 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ANSET4ZE25TS2BEWIKSMDFT2OYOXTAVCNFSM6AAAAABV6JWYSWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNBVG44DENJWGM>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
I have just done some tests (still need to add pipeline tests)
|
I think an exception for killing the primary is a very nice idea.
Is it better just to stop the application? |
Adresses #218.
Not tested so far. I did try the
make remage-doc-dump
as i add new macro commands here, but it did not update thermg-commands
file.@tdixon97 I made a new PR instead of re-opening the old one, as i force pushed the changes back then into non-existence.