Conversation
GuySten
changed the title
GuySten
marked this pull request as ready for review
December 9, 2025 23:55
paulromano
requested changes
Dec 12, 2025
paulromano
left a comment
paulromano
left a comment
Contributor
There was a problem hiding this comment.
@GuySten Thanks for starting with a small PR to get us toward support for photonuclear physics. This PR just needs some documentation but otherwise it looks good to me.
Contributor
Author
|
This PR should be ready now. |
Contributor
Author
|
@paulromano, can you take some time to finish reviewing this PR? |
shimwell
reviewed
Aug 24, 2026
shimwell
reviewed
Aug 24, 2026
Co-authored-by: Jonathan Shimwell <drshimwell@gmail.com>
Co-authored-by: Jonathan Shimwell <drshimwell@gmail.com>
Contributor
Author
|
Thanks for reviewing, @shimwell. |
Replacing the `threshold`/`mass_ratio` attributes of a `level` distribution
with `q_value`/`mass`/`particle` is a format change in the direction nobody
tests: this branch reads old files, but nothing makes an old OpenMC read a
file written here. Three things combined to make that silent rather than
noisy.
- The writer emitted only the new attributes.
- HDF5_VERSION stayed 3.0 on both sides, so check_data_version accepted the
file.
- read_attr() ignores the status of H5Aopen and H5Aread, so a missing
attribute leaves the caller's buffer untouched and the run continues.
The consequence is not confined to photonuclear work. LevelInelastic is MT=51
through MT=90 of every neutron nuclide, so any NEUTRON library regenerated
with this branch's openmc.data is read by a released OpenMC 3.x from
uninitialised memory, with photonuclear physics switched off entirely.
Measured on a 14 MeV point source in an aluminium sphere, where MT=51 is the
dominant non-elastic channel, 5x4000 histories, OMP_NUM_THREADS=1:
tally correct old OpenMC + new-format library
flux 2.00645e+01 1.84649e+01 -8.0 %
(n,elastic) 2.03068e+00 1.53570e+00 -24.4 %
absorption 1.45594e-01 1.37280e-01 -5.7 %
heating 2.09237e+06 1.99514e+06 -4.6 %
Exit code 0, a normal statepoint, and nothing on stderr but HDF5-DIAG noise
that is easy to miss in a batch job.
What this commit does
- to_hdf5 writes `threshold` and `mass_ratio` alongside the new attributes
whenever the projectile is a neutron. The two forms describe the same law,
so an older reader gets the right answer. There is no legacy form for a
photon, but an older reader has no photonuclear support either, so it
never sees one.
- Both readers now prefer the q_value form, since a file written here
carries both and only that form records the projectile.
- HDF5_VERSION is 3.1 in openmc/data/__init__.py and constants.h, because
the attribute set did change, and check_data_version refuses a minor
version newer than the build rather than reading what it can.
- read_attr fails loudly on a missing or unreadable attribute. This is the
mechanism that made the mismatch invisible, and it covers every caller:
read_attribute() had no existence check at all.
Also here, from the same review: LevelInelastic.from_ace raised a bare
KeyError for any ACE table type other than 'c' or 'u', and when the mass
recovered from mass_ratio disagreed with the table's atomic weight ratio it
warned and silently substituted the latter -- converting the one available
self-consistency check into a warning nobody reads. It now raises, at a
tolerance that reflects how much inverting mass_ratio amplifies the error
(roughly (A+1)/2, a factor of ~120 for A=238, so the default rel_tol of 1e-9
is too tight for a consistent file).
Verified: with this build, the same problem gives bitwise identical results
from a library with the legacy attributes and one without, so the new law
reproduces the old one exactly; and an older OpenMC reading a library written
by this writer now matches this build bitwise, where it was 24 % off before.
tests/unit_tests/test_data_level_inelastic.py pins both directions of the
contract, including that a neutron file keeps the legacy attributes and a
photon file does not.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RRQZN4MbeFsELiufinhvZ6
Older versions of NJOY wrote the LAW=3 (level inelastic) parameters of a photonuclear table with neutron kinematics -- threshold = (A + 1)/A |Q| and mass_ratio = (A/(A + 1))^2 -- rather than the photonuclear form the reader expects, threshold = |Q| and mass_ratio = (A - 1)/A. Those tables are what the photonuclear libraries in circulation are made of. Read with the photonuclear convention, such a mass_ratio implies a target mass of (A + 1)^2/(2A + 1), about A/2, so the mass consistency check fails. That check used to warn and substitute the table's atomic weight ratio; since 96ed80f it raises, which takes photonuclear library generation down entirely. from_ace now tells the two conventions apart instead. For a photonuclear table whose mass_ratio does not match the atomic weight ratio under the photonuclear form, it tries the neutron form; if that one matches, it warns and reads both words with that convention, which recovers |Q| exactly (threshold*sqrt( mass_ratio) = |Q|). The distribution is still built with particle='photon', since the projectile comes from the table type, so transport applies the photonuclear kinematics of the level scattering law -- only the reading of the two stored words changes. A mass_ratio matching neither convention is a broken or misparsed table and still raises, now naming the mass ratio and both readings of it. The inversion is also guarded against a mass_ratio outside the range each convention can produce, which previously gave a ZeroDivisionError or a math domain error from the wrong-convention attempt rather than a diagnosable one. Tests cover both conventions on both table types, the warning and exact Q-value recovery for a legacy photonuclear table, an inconsistent mass ratio, and a non-invertible one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WtqukonxdorWTf1zDAs394
This branch has not been deployed
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
4 participants
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.
Description
This PR extend Level Scattering to support incident photons.
This is a small step in implementing photonuclear physics.
A roadmap for the photonuclear physics feature can be found here #1941.
Checklist