-
Notifications
You must be signed in to change notification settings - Fork 45
Support material-dependent model applicability #1790
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: develop
Are you sure you want to change the base?
Conversation
Test summary 4 395 files 6 730 suites 4m 18s ⏱️ Results for commit 2a2f091. |
|
@sethrj I don't plan on resuming work on this near term, but it might be good to clear up the motivation for this and whether we really need it. |
|
Thanks @amandalund , this one's completely slipped my mind (sincere apologies). It's likely another case of overengineering. And either way, it's a subtle physics bug fix rather than an important critical path like the other optical/muon work being done. The main goal is to provide a unified way to evaluate the minimum energy threshold for the many models that require it, including for disabling the "discrete" part of the continuous-discrete interactions like ionization below the incident energy corresponding to the secondary production threshold. This would also get rid of all the As a result, tracks that slow down below a threshold over their step wouldn't "miss" the interaction and keep going; they'd only sample from the applicable processes/models. What we're doing now is wrong, and I think the change to precalculating process applicability would make the process selection consistent with the integral XS rejection. As a secondary benefit it would allow us to move more duplicate code out of models and into the central "interaction physics" component. This mostly affects code cleanliness and "performance" but as we know the performance for the physics part of EM physics is relatively trivial. |
|
Thanks @sethrj! And no worries, we discussed deferring this a while back.
I worry calculating and storing the model limits is more trouble than it's worth. It's not trivial to do, but I’m also not sure we need it.
We currently don't require material-dependent applicability to correctly sample continuous vs. discrete interactions based on the production threshold. Those checks in SB and BH were added in #1727 specifically for when the integral approach is disabled. With integral on, we sample a process using the pre-step energy/xs, recalculate that process xs at the post-step energy and accept the interaction with probability xs_post / xs_max. If the cross section at the post-step is zero (e.g. if we should apply continuous energy loss rather than a discrete interaction) no interaction occurs. When it’s disabled, we sample a process using the pre-step energy, find the model for that process that applies at the post-step energy and then apply that interaction even if the track’s energy is below the limit for the production cut. With material-dependent applicability we can in the latter case reject the model if it doesn't apply and eliminate those checks, which is why they are removed here. But we're still rejecting the interaction.
How? Would we recalculate each process xs post step and sample from those (but that would be biased?)?
I think I'm still missing something... I thought what we’re doing currently is correct when using the integral approach. |
|
I think I'm missing something too 😔 but I trust your judgment here and let's close this and the other issue. We can probably vastly simplify the applicability code too. I'd hoped the model work could be applicable to #1898 too but I doubt that's how we'll do it. |
Continuing with #907, I've started adding support for material-dependent model bounds. However, I'm not completely convinced we need or want this... it feels a little like we're adding extra work and complexity trying to explicitly specify something that's already implicitly handled. @sethrj before I keep going on this it could be helpful to discuss the motivation for it/why it's necessary.