Skip to content
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

Refactor rejection sampling using a helper class #1354

Merged
merged 8 commits into from
Aug 6, 2024

Conversation

sethrj
Copy link
Member

@sethrj sethrj commented Aug 5, 2024

This defines a RejectionSampler class that can be used as a more intuitive replacement for BernoulliDistribution during rejection sampling loops. Instead of "accepting" when the sample is within bounds (necessitating an easy-to-miss negation in the while condition) we "reject" by returning true unless the sampled value is accepted.

@sethrj sethrj added physics Particles, processes, and stepping algorithms minor Minor internal changes or fixes labels Aug 5, 2024
@sethrj sethrj requested a review from amandalund August 5, 2024 16:04
```
Internal error: assertion failed at: "overload.c", line 28946 in deduce_class_template_args

1 catastrophic error detected in the compilation of "/__w/celeritas/celeritas/src/celeritas/em/model/MollerBhabhaModel.cu".
```
Copy link
Contributor

@amandalund amandalund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sethrj! Hopefully this helps prevent any more rejection sampling bugs going forward.

@sethrj sethrj enabled auto-merge (squash) August 6, 2024 03:05
@sethrj sethrj merged commit f2a9640 into celeritas-project:develop Aug 6, 2024
29 checks passed
@sethrj sethrj deleted the rejection-sampler branch August 6, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Minor internal changes or fixes physics Particles, processes, and stepping algorithms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants