-
Notifications
You must be signed in to change notification settings - Fork 34
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 atomic relaxation #134
Conversation
Kickass results @amandalund ! |
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.
Excellent work. I'm a little uneasy about how we're combining multiple setup-time decisions (no atomic relaxation, relaxation without auger electrons, full relaxation) into low-level runtime conditionals. It's sort of an optimization, so we don't have to worry about it right now, but I think for situations like this the Model and Interactor classes could be templated based on the behavior (maybe something as simple as an enum defined in the Interface
class, which can be used with some traits classes to move around the necessary data), then the setup-time Process::build_models
would emit a different Model
based on whether the selection.
src/physics/em/AtomicRelaxation.hh
Outdated
|
||
// Simulate atomic relaxation with an initial vacancy in the given shell ID | ||
template<class Engine> | ||
inline CELER_FUNCTION size_type operator()(size_type shell_id, Engine& rng); |
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.
shell_id
should be part of the constructor -- "distribution"-like functors should take the RNG as their only element.
Looking at the implementation of AtomicRelaxation
, we might want to add a new small helper class to emit the distribution, which can also connect the dots for "how many secondaries to allocate" and "how to allocate and return gracefully if there's not enough room". Maybe something like this would go into the interactor body:
Span<Secondary> secondaries;
if (AtomicRelaxationHelper relax_helper{shared, el_id, shell_id})
{
// Need to do relaxation (define operator bool() as num_secondaries > 0)
secondaries = relax_helper.allocate(allocate_secondaries_);
if (secondaries.empty())
{
// Failed to allocate
return Interaction::from_failure();
}
// Create the sampler from the allocated secondaries
AtomicRelaxation sample_relaxation = relax_helper.build_distribution(secondaries);
// Sampler returns the subspan of created secondaries
auto num_secondaries = sample_relaxation(rng);
CELER_EXPECT(num_secondaries > 0);
// TODO: I need to implement "first" and "last" in Span
secondaries = secondaries.first(num_secondaries);
}
{ | ||
interface_.atomic_relaxation = celeritas::is_device_enabled() | ||
? atomic_relaxation.device_pointers() | ||
: atomic_relaxation.host_pointers(); |
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.
For now, I think the model interface needs to be device-only since interact
always assumes device-only. If and when we want to support transport on the CPU we'll need to implement another interact
that takes host pointers.
I still have a little work to do here @sethrj (helper class to emit the distribution, possibly address your concerns about the setup-time decisions, which I definitely agree with -- uneasy is the right word 😉), but thanks for your feedback so far! |
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.
Looking good! A couple more comments.
std::ifstream infile(filename); | ||
CELER_VALIDATE(infile, "Couldn't open '" << filename << "'"); | ||
|
||
int des = 0; |
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.
Not gonna lie, every time I read this variable name I think of Karl Urban's character in Thor: Ragnarok 😂
https://getyarn.io/yarn-clip/54bf355e-34fd-4d10-8e82-7a3564ebb1c7
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.
Still true a year and a half later 🤣
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.
Nice work @amandalund !
@amandalund Ready for merge, right? |
If you say so @sethrj! Thanks as always for the helpful feedback. |
This adds the ability to simulate the emission of fluorescence photons and Auger electrons when a primary process produces a vacancy in an atomic shell.