Generalize Poisson distribution to allow zeros#2408
Open
sethrj wants to merge 32 commits into
Open
Conversation
sethrj
added
physics
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends PoissonDistribution to support the degenerate case lambda == 0 (always returning zero without RNG usage) and refactors several optical/EM sampling sites to take advantage of that behavior, reducing branching and avoiding unnecessary work in “no photons” cases.
Changes:
- Generalize
PoissonDistributionto allowlambda >= 0, add alambda==0method path, and clamp Gaussian-approximation results to nonnegative values. - Simplify WLS/Cherenkov offload sampling by always calling the sampler and relying on
PoissonDistribution{0}to yield zero. - Update EM excitation-loss sampling to no longer special-case
xs_exc_ > 0(relying on Poisson(0) behavior).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/corecel/random/distribution/PoissonDistribution.test.cc | Adds a regression test for lambda==0 returning zeros and consuming no RNG samples. |
| src/corecel/random/distribution/PoissonDistribution.hh | Adds explicit lambda==0 path and refactors method selection/state; clamps Gaussian approximation output. |
| src/celeritas/optical/interactor/WavelengthShiftInteractor.hh | Uses PoissonDistribution{0} to represent “no emission” and samples unconditionally in operator(). |
| src/celeritas/optical/gen/ScintillationOffload.hh | Adjusts Gaussian rounding approach for photon counts while keeping clamping behavior. |
| src/celeritas/optical/gen/CherenkovOffload.hh | Stores a precomputed PoissonDistribution member for repeated sampling and removes early-return branch. |
| src/celeritas/em/distribution/EnergyLossUrbanDistribution.hh | Removes xs_exc_ > 0 guard, relying on Poisson(0) to yield zero collisions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Test summary 5 715 files 9 303 suites 18m 47s ⏱️ For more details on these failures, see this check. Results for commit 43089a2. ♻️ This comment has been updated with latest results. |
amandalund
approved these changes
May 21, 2026
Implement a templated RoundedNonnegDistribution that wraps an underlying floating-point distribution and returns rounded non-negative integers. Add focused unit tests for rounding/clamping behavior, one-underlying-sample usage, and deterministic uniform sampling output. Register the new test in corecel CMake test definitions. Prompt: "Using #sym:TruncatedDistribution as a template, create a RoundedNonnegDistribution that efficiently samples from an underlying distribution and returns a non-negative integer. Template parameters should be underlying distribution and integer type (default to celeritas::size_type)." Assisted-by: GitHub Copilot (GPT-5.3-Codex)
amandalund
approved these changes
May 22, 2026
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #2408 +/- ##
========================================
Coverage 87.19% 87.19%
========================================
Files 1399 1400 +1
Lines 44114 44150 +36
Branches 13314 13317 +3
========================================
+ Hits 38465 38497 +32
- Misses 4414 4416 +2
- Partials 1235 1237 +2
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The current Poisson already has a special case for large N (using Gaussian); this cleans up the code a bit by extending it with another special case for N=0, and avoiding extra computations based on which case.