Skip to content

Photonuclear Physics part 1 #3439

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

Open
wants to merge 21 commits into
base: develop
Choose a base branch
from

Conversation

GuySten
Copy link
Contributor

@GuySten GuySten commented Jun 11, 2025

Description

This pull request is the first step of implementing photonuclear physics in OpenMC.
This pull request handle definition and conversion of nuclear data from endf/ACE formatted libraries to hdf5 format.
Because photonuclear physics is relatively a big feature, I feel that breaking it into chunks is preferable.
I am open to constructive criticism regarding code structure, documentation, testing, etc.

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)

@shimwell
Copy link
Member

Wow super to see work on this, it sounds like the PR would help fix a long standing issue of not being able to process TENDL gamma cross sections.
#1941

@@ -20,6 +20,7 @@ class Library {
enum class Type {
neutron = 1,
photon = 3,
photonuclear = 6,
Copy link
Contributor

Choose a reason for hiding this comment

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

@paulromano is this list out of order to test neurodivergents?

projectile_mass = ev.projectile["mass"]
if ev.projectile['mass'] == 0.0:
projectile_za = 0
elif np.isclose(ev.projectile['mass'], 1.0, atol=1.0e-12, rtol=0.):
Copy link
Contributor

Choose a reason for hiding this comment

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

are these tolerances set uniformly across the data module? Maybe this should be elevated to some constant somewhere that is globally accessible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those tolerances were originaly from a check in openmc/data/kalbach_mann.py.
So the only thing I did was to move code around.

I do not think that setting global tolerance variables will improve code readability so I am against this suggested change.

@GuySten
Copy link
Contributor Author

GuySten commented Jun 11, 2025

@makeclean Thanks for your review and suggested changes, most of which I accepted.

After including some tests I think this pr is ready for formal review.

@GuySten GuySten marked this pull request as ready for review June 11, 2025 20:11
@GuySten GuySten requested a review from paulromano as a code owner June 11, 2025 20:11
@GuySten GuySten force-pushed the photonuclear-physics-pt1 branch 2 times, most recently from f55d2ed to 6f8df6b Compare June 12, 2025 18:24
Copy link
Contributor

@ahnaf-tahmid-chowdhury ahnaf-tahmid-chowdhury left a comment

Choose a reason for hiding this comment

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

Thanks for the excellent work! I have a few suggestions.

@@ -3,11 +3,11 @@ set -ex

# Download HDF5 data
if [[ ! -e $HOME/nndc_hdf5/cross_sections.xml ]]; then
wget -q -O - https://anl.box.com/shared/static/teaup95cqv8s9nn56hfn7ku8mmelr95p.xz | tar -C $HOME -xJ
wget -q -O - https://github.com/GuySten/data/releases/download/v1.0.0/nndc_hdf5_test.tar.xz | tar -C $HOME -xJ
Copy link
Contributor

Choose a reason for hiding this comment

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

The photonuclear data for Pu239.h5 is currently unavailable from ANL, isn't it? Perhaps we should consider serving this from ANL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@paulromano, I leave the decision to you.

@pytest.fixture(scope='module')
def pu239():
"""Pu239 HDF5 data."""
directory = os.path.dirname(os.environ['OPENMC_CROSS_SECTIONS'])
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we consider using openmc.config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because ENDF location is not in openmc.config I think this way is more consistent.
I would change my opinion if openmc.config would include ENDF location.

Copy link
Contributor

Choose a reason for hiding this comment

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

Feel free to suggest any feathers here PR #3461.

@GuySten
Copy link
Contributor Author

GuySten commented Jun 14, 2025

@ahnaf-tahmid-chowdhury Thanks for your review! You noticed some things that I missed.

Currently the ci tests crash on the test:
tests/regression_tests/deplete_decay_only/test.py::test_decay_only[coupled]
This crash also happens to me when I test locally so I will update this branch when I will figure out what happens.

@GuySten
Copy link
Contributor Author

GuySten commented Jun 14, 2025

I fixed a bug and now all tests pass.

@GuySten GuySten force-pushed the photonuclear-physics-pt1 branch from 26ad64b to 8722e9b Compare June 17, 2025 08:46
@GuySten GuySten force-pushed the photonuclear-physics-pt1 branch from 8722e9b to a2fbdf5 Compare June 26, 2025 11:44
@GuySten GuySten force-pushed the photonuclear-physics-pt1 branch from a2fbdf5 to b802e4a Compare July 21, 2025 07:53
@GuySten GuySten force-pushed the photonuclear-physics-pt1 branch from af9d98a to c13c6a2 Compare July 23, 2025 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants