Fix point detector scoring: activate tallies, set ParticleRay state, fix Python reader#39
Merged
GuySten merged 2 commits intoGuySten:point-detectorfrom Apr 30, 2026
Conversation
Three bugs prevented point detector tallies from producing any results: 1. setup_active_tallies() had no case for TallyType::POINT, so active_point_tallies and active_point_detectors were never populated. All scoring hooks guarded by 'if (!active_point_tallies.empty())' silently skipped every event. 2. score_point_tally_impl() did not set ParticleRay position/energy state before calling score_tracklength_tally_general(). PointFilter requires p.r() == detector position and EnergyFilter reads p.E_last(), both of which were uninitialized/wrong after Ray::trace(). 3. openmc_statepoint_write() had no branch for TallyEstimator::NEXT_EVENT, so the 'estimator' dataset was missing from the HDF5 output, causing Python StatePoint reader to crash with KeyError. Fixes: - Add TallyType::POINT case in setup_active_tallies() that pushes to active_point_tallies and inserts detector positions from PointFilter. - Add corresponding clear() calls in setup_active_tallies() and free_memory_tally(). - Include filter_point.h in tally.cpp. - Set p.r(), p.r_last(), and p.E_last() on the ParticleRay before scoring. - Write 'next-event' estimator string in statepoint for NEXT_EVENT tallies. Verified with smoke tests: water sphere with point detectors produces non-zero flux, 1/r^2 scaling matches analytical expectation within 2%, and symmetric detectors yield statistically consistent results.
…type PointFilter lacked from_hdf5() and get_pandas_dataframe() overrides. The base Filter.from_hdf5() passed a flat numpy array to PointFilter.__init__ which expects Sequence[tuple], causing TypeError. The base get_pandas_dataframe() used np.repeat on the nested tuple structure, causing ValueError. Also add 'next-event' to ESTIMATOR_TYPES so the Tally.estimator setter accepts the value read from statepoint files. Fixes: - Add PointFilter.from_hdf5() that reconstructs (pos, r0) tuples from flat HDF5 bins array. - Add PointFilter.get_pandas_dataframe() that formats detector positions as readable string labels. - Add 'next-event' to ESTIMATOR_TYPES set in tallies.py.
Author
|
Hi @GuySten, just checking in — do you have a chance to review this PR? The fixes here are needed to get point detector tallies scoring correctly (currently all results are zero without these changes). Happy to discuss or adjust anything. Thanks! |
Owner
|
Currently I do not have sufficient time to dive into the point detector feature. |
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
2 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.
Summary
This PR fixes 4 bugs that completely prevented point detector tallies from producing any results on the
point-detectorbranch.Bug 1:
active_point_tallies/active_point_detectorsnever populatedsetup_active_tallies()intally.cpphad cases forVOLUME,MESH_SURFACE,SURFACE,PULSE_HEIGHT— but nocase TallyType::POINT. Bothactive_point_talliesandactive_point_detectorsremained empty. All scoring hooks guarded byif (!model::active_point_tallies.empty())in physics.cpp/source.cpp silently skipped every event.Bug 2: Statepoint missing
estimatordataset forNEXT_EVENTThe if/else chain in
openmc_statepoint_write()only handled ANALOG/TRACKLENGTH/COLLISION. Noestimatordataset was written for point tallies, causing the PythonStatePointreader to crash withKeyError.Bug 3:
ParticleRayposition/energy state not set for filter matchingIn
score_point_tally_impl(), afterRay::trace(), theParticleRay'sr()was at whatever geometry exit point the ray reached — NOT at the detector position.PointFilter::get_all_bins()requiresp.r() ≈ detector(line 45 check:(p.r() - pos).norm() < FP_COINCIDENT). Also,E_last()was uninitialized, soEnergyFilter::get_all_bins()read garbage.Bug 4: Python
PointFiltermissingfrom_hdf5andget_pandas_dataframeBase class
from_hdf5()passed a flat numpy array toPointFilter.__init__()which expectsSequence[tuple]→TypeError. Baseget_pandas_dataframe()callednp.repeat()on nested tuples →ValueError.Testing
Files changed
src/tallies/tally.cpp— AddTallyType::POINTcase + clear() calls + includeinclude/openmc/tallies/tally_scoring.h— Setr(),r_last(),E_last()on ParticleRaysrc/state_point.cpp— Writenext-eventestimator stringopenmc/filter.py— AddPointFilter.from_hdf5()andget_pandas_dataframe()openmc/tallies.py— Addnext-eventtoESTIMATOR_TYPES