[Point Detector] Add distribution get_pdf functionality #3550

itay-space · 2025-08-27T15:46:18Z

Description

This PR is the first step in breaking down the point detector tally project into smaller, reviewable pieces, following the large PR #3109
and the paper Development and benchmarking of a point detector in OpenMC.

As suggested during the earlier discussion (thanks @GuySten, @gridley, @shimwell !), the natural starting point is to add the ability to correctly calculate PDFs on the C++ side.

Accordingly, this PR introduces the get_pdf implementations for the distribution classes. This is essentially a clean cut from the original large PR, with the goal of keeping the scope focused and review manageable.

We plan to follow up with tests and additional pieces of the point detector work in subsequent PRs.

Checklist

I have performed a self-review of my own code
I have run clang-format (version 15) on any C++ source files (if applicable)
I have followed the style guidelines for Python source files (if applicable)
I have made corresponding changes to the documentation (if applicable)
I have added tests that prove my fix is effective or that my feature works (if applicable)

GuySten

Nice work @itay-space!

My review focus more on the code structure side of things.
I did not check the maths.

include/openmc/distribution.h

include/openmc/particle_data.h

src/physics.cpp

src/secondary_correlated.cpp

src/secondary_kalbach.cpp

src/secondary_thermal.cpp

src/thermal.cpp

Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>

src/distribution.cpp

include/openmc/angle_energy.h

include/openmc/reaction_product.h

src/distribution.cpp

src/distribution_angle.cpp

src/reaction_product.cpp

src/secondary_correlated.cpp

src/secondary_nbody.cpp

Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>

include/openmc/angle_energy.h

include/openmc/distribution_angle.h

include/openmc/secondary_correlated.h

include/openmc/secondary_kalbach.h

include/openmc/secondary_correlated.h

src/secondary_thermal.cpp

src/secondary_uncorrelated.cpp

src/thermal.cpp

vendor/gsl-lite

src/reaction_product.cpp

Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>

itay-space · 2025-09-25T04:53:33Z

@GuySten , Thanks for the suggestion! These classes already rely on two independent 1D distributions (polar and azimuthal), and the evaluate methods are implemented at that level. The sphere distributions just combine those underlying 1D PDFs, so the functionality is already covered.

GuySten · 2025-09-25T05:37:19Z

@itay-space, the conversion is not trivial because of the reference direction, so I think that adding this functionality is worth while.
If you do not object I can add the relevant functionality myself.

itay-space · 2025-09-25T05:43:41Z

@GuySten I’m not sure why this would be different from the other cases (I can’t see it right now), but sure, go ahead and add the functionality. I have no objections. Thanks!

GuySten · 2025-09-28T16:52:44Z

I am waiting for #3582 to be merged to add the functionality I talked about.
Without that PR it is unclear what is the correct mapping between a point on the unit sphere and the mu, phi angles to calculate the pdf for.

GuySten · 2025-10-14T22:41:01Z

@paulromano, I am reminding you to look at this PR when you have some time.

shimwell · 2025-11-03T08:21:36Z

I am waiting for #3582 to be merged to add the functionality I talked about. Without that PR it is unclear what is the correct mapping between a point on the unit sphere and the mu, phi angles to calculate the pdf for.

Thanks for breaking out this small PR and great to see it is now merged

GuySten · 2025-11-03T17:39:42Z

I've added the functionality I wanted.

@paulromano, could you take some time to review this PR? You asked to review this RP before we are merging it, and I want to merge this PR and continue to the next parts of the point detector work.

paulromano · 2025-11-03T17:44:33Z

@GuySten Yes, I will try to get to it this week

paulromano

@itay-space @GuySten Just wanted to let you know I am starting to go through this PR. It's not as simple as I was hoping as there is quite a bit of effectively duplicated code in the AngleEnergy classes. I'll see what I can do with it but let me know if either of you have ideas.

GuySten · 2025-12-03T22:09:26Z

I simplified this PR significantly.
@paulromano, are these changes enough?

GuySten · 2026-01-06T15:50:28Z

@paulromano, could you take some time to look at this PR?
It is in much better shape now.

paulromano · 2026-01-06T23:02:21Z

Thanks for the ping @GuySten. I'm working through PR reviews this week and will try to get to this one.

paulromano · 2026-01-16T06:42:09Z

src/secondary_thermal.cpp

+  vector<double> energies_cut(energies.begin(), energies.begin() + i + 1);
+  vector<double> factors_cut(factors.begin(), factors.begin() + i + 1);
+
+  vector<double> mu_vector_rev;
+  std::transform(energies_cut.begin(), energies_cut.end(),
+    std::back_inserter(mu_vector_rev),
+    [E_in](double Ei) { return 1 - 2 * Ei / E_in; });
+  vector<double> mu_vector(mu_vector_rev.rbegin(), mu_vector_rev.rend());
+
+  auto f = xt::adapt(factors_cut, {
+                                    factors_cut.size(),
+                                  });
+  auto weights = xt::diff(f);
+  weights /= xt::sum(weights);
+  vector<double> w(weights.begin(), weights.end());


The use of std::vector here results in dynamic memory allocation, which is undesirable within any code that appears within the transport loop since it is threaded. @itay-space @GuySten can you guys see if you can restructure this to avoid the use of new vector objects?

GuySten · 2026-01-16T11:01:31Z

@paulromano, I've managed to reduce the number of vector allocations to 2.
Currently, I have no idea how to reduce it further.

GuySten · 2026-01-16T12:22:46Z

@paulromano, I have some ideas of how to reduce the memory allocations further
but it will be easier to test them once we have reached the point where we have point detector regression tests.

Are you okay with merging it as is and open an issue to improve memory allocations in the future?

Implement get_pdf functionality

06ae298

itay-space changed the title ~~Implement get_pdf functionality~~ [Point Detector] Add distribution get_pdf functionality Aug 27, 2025

GuySten requested changes Aug 27, 2025

View reviewed changes

GuySten added Other Enhancement Point Detector and removed Other Enhancement labels Aug 27, 2025

itay-space and others added 6 commits August 28, 2025 08:32

Update include/openmc/distribution.h

78d841e

Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>

Update include/openmc/particle_data.h

33a27a9

Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>

Update include/openmc/particle_data.h

d968875

Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>

Update src/physics.cpp

3b896c0

Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>

Update src/physics.cpp

c034bff

Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>

Update src/physics.cpp

a5d2f4c

Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>

GuySten reviewed Aug 28, 2025

View reviewed changes

src/distribution.cpp Outdated Show resolved Hide resolved

Your Name and others added 5 commits August 31, 2025 14:50

Cleaning some code

ca5b44e

Resolving conversations

eed8b1c

cosmetic fixes

f2ecdd4

Merge branch 'develop' into get_pdf

71d5fa2

simplified code significantly

c3b4d82

GuySten requested changes Aug 31, 2025

View reviewed changes

itay-space and others added 8 commits August 31, 2025 18:06

Update src/distribution_angle.cpp

78a33cc

Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>

ran clang format

6d70d39

fixes

acfd402

cosmetics

98ca32e

using openmc's fatal_error

26d2a56

Use accessors in powerlaw

7abe69c

clausing error for different distributions

d183cce

fixing function signature

37b41c4

GuySten requested changes Sep 1, 2025

View reviewed changes

GuySten reviewed Sep 1, 2025

View reviewed changes

src/reaction_product.cpp Outdated Show resolved Hide resolved

Update include/openmc/angle_energy.h

007dcf5

Co-authored-by: GuySten <62616591+GuySten@users.noreply.github.com>

GuySten added 3 commits November 3, 2025 17:45

Merge branch 'develop' into get_pdf

b0613dc

Merge branch 'develop' into get_pdf

8bb4356

added evaluate for SphericalUnitDistribution

cc7c3a5

paulromano requested changes Nov 12, 2025

View reviewed changes

GuySten added 2 commits December 3, 2025 23:05

Merge branch 'develop' into get_pdf

758a01f

simplify PR significantly

481901e

GuySten requested a review from paulromano December 8, 2025 13:29

Merge branch 'develop' into get_pdf

b067e7f

some fixes

43a6fbb

GuySten force-pushed the get_pdf branch from e4c9a76 to 43a6fbb Compare January 12, 2026 17:52

paulromano requested changes Jan 16, 2026

View reviewed changes

further simplification

c52ad4f

GuySten added 3 commits January 16, 2026 13:04

further simplification

f9d6d0f

further simplification

dc38dc5

reserve vector sizes

02aea31

GuySten and others added 2 commits January 17, 2026 18:35

Fix mu_vector reservation size in secondary_thermal.cpp

f14005a

clang format

903af78

[Point Detector] Add distribution get_pdf functionality #3550

Are you sure you want to change the base?

[Point Detector] Add distribution get_pdf functionality #3550

Uh oh!

Conversation

itay-space commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

GuySten left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

itay-space commented Sep 25, 2025

Uh oh!

GuySten commented Sep 25, 2025

Uh oh!

itay-space commented Sep 25, 2025

Uh oh!

GuySten commented Sep 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GuySten commented Oct 14, 2025

Uh oh!

shimwell commented Nov 3, 2025

Uh oh!

GuySten commented Nov 3, 2025

Uh oh!

paulromano commented Nov 3, 2025

Uh oh!

paulromano left a comment

Choose a reason for hiding this comment

Uh oh!

GuySten commented Dec 3, 2025

Uh oh!

GuySten commented Jan 6, 2026

Uh oh!

paulromano commented Jan 6, 2026

Uh oh!

paulromano Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

GuySten commented Jan 16, 2026

Uh oh!

GuySten commented Jan 16, 2026

Uh oh!

Reviewers

itay-space commented Aug 27, 2025 •

edited

Loading

GuySten left a comment •

edited

Loading

GuySten commented Sep 28, 2025 •

edited

Loading