Fix double filter weighting of delayed-nu-fission with EnergyoutFilter and DelayedGroupFilter

[ondrejch]

Description

score_fission_eout() applied the filter weights twice when it scored a delayed fission neutron on a tally that has both an EnergyoutFilter and a DelayedGroupFilter. It multiplied the score by the product of the filter weights, then passed the result to score_fission_delayed_dg(), which multiplies by the same product again. As a result, every filter with non-unit weights on such a tally was applied twice: a score that should be multiplied by the weight w was multiplied by w^2.

This PR removes the multiplication at the call site. The weights are now applied once, inside the helper, as for every other caller. The PR also adds a regression test that needs no nuclear data. The bug was found while developing a time-moment tally filter (see "Related PRs").

Fixes #4181

Motivation

Affected tallies. The bug affects analog delayed-nu-fission tallies that combine all three of:

• an EnergyoutFilter,
• a DelayedGroupFilter,
• a filter with non-unit weights, for example EnergyFunctionFilter (continuous energy only) or the expansion filters (LegendreFilter, SphericalHarmonicsFilter, SpatialLegendreFilter, ZernikeFilter, ZernikeRadialFilter).

With an EnergyoutFilter, these scores always use the analog estimator. Continuous-energy and multigroup runs both take this path.

Unaffected tallies:

• Tallies whose filters all have unit weights, such as cell, material, mesh and energy filters. This includes the delayed MGXS tallies built by openmc.mgxs (for example ChiDelayed and DelayedNuFissionMatrixXS).
• nu-fission and prompt-nu-fission with an EnergyoutFilter.
• delayed-nu-fission with an EnergyoutFilter but no DelayedGroupFilter.
• delayed-nu-fission without an EnergyoutFilter.

History. The call site's own multiplication was correct until d5a2340 ("Consistent use of filter_weight", merged in #1462 and first released in v0.12.0). That commit moved the weight product into score_fission_delayed_dg() but did not remove it from this caller. Every release since v0.12.0, and develop up to a5bc348, applies the weights twice on this path.

Example (continuous energy). A UO2/water pin cell with an EnergyFunctionFilter of constant value 2, measured on develop at 3cded0f, before and after this change, with ENDF/B-VII.1 data (a check outside the test suite):

Ratio	Before	After	Expected
[EO, DG, EF] / [EO, DG]	4.0	2.0	2
[EO, DG, EF] / [DG, EF] (analog, no EnergyoutFilter)	2.0	1.0	1

The "after" ratios are exact.

Design

The change deletes the loop that computed the weight product at the call site and passes score unchanged. score_fission_delayed_dg() already computes both the filter index (after swapping in the delayed group bin) and the product of all filter weights, so the loop was redundant. A comment at the call site now says the helper applies the weights.

The opposite fix, keeping the multiplication at the call site and removing it from the helper, would break the helper's 15 other callers. All of them pass unweighted scores and rely on the helper to apply the weights. After this change, every call site in tally_scoring.cpp applies the weights exactly once.

The fix also removes a loop over the tally filters for each banked delayed neutron, so this path does slightly less work.

Tests

New regression test: tests/regression_tests/mg_delayed_eout_weights

Model. The test uses openmc.examples.slab_mg(): a 929.45 cm multigroup slab, reflective at x = 0 and vacuum at x = 929.45 cm, run with 1000 particles for 10 batches (5 inactive). The test writes a two-group library, 2g.h5:

• The cross sections are those of mg_tallies.
• Two delayed groups are added with unphysically large delayed fractions (0.1 and 0.2), so a short run banks many delayed neutrons.
• The delayed spectra populate both groups, so both outgoing energy bins are scored.

Tallies. The test defines three analog delayed-nu-fission tallies that score the same delayed neutrons. SL is SpatialLegendreFilter(1, 'x', -929.45, 929.45), so the P1 weight is x/929.45, which lies between 0 and 1 inside the slab. The analog estimator is set explicitly on all three tallies: without it, the SpatialLegendreFilter would give the delayedgroup tally a collision estimator, and the checks below rely on all three tallies scoring the same analog events. A comment in the test says so.

Tally	Filters	Scoring path
energyout-delayedgroup	[EO, DG, SL]	score_fission_eout() delayed-group branch (the one that was wrong)
delayedgroup	[DG, SL]	score_general_mg() analog delayed-group path (no EnergyoutFilter)
energyout	[EO, SL]	score_fission_eout() branch without a delayed group filter

Checks. Each check compares two tallies that score the same events through different code paths, so they must agree to round-off:

• Summing the first tally over the outgoing-energy bins must equal [DG, SL], within rtol=1e-10.
• Summing the first tally over the delayed groups must equal [EO, SL], within rtol=1e-10.
• A guard requires every bin of both reference tallies to be positive, so the comparison cannot pass vacuously.
• The usual inputs_true.dat and results_true.dat comparison then runs.

All P1 weights are non-negative, so the P1 sums do not cancel and a relative tolerance is meaningful. The observed deviation after the fix is at most 1.6e-15. The P0 bins have unit weight and agree even without the fix, so the P1 bins are what detect the bug.

Harness choice. The test subclasses PyAPITestHarness, as the other Python-API MG tests do:

• inputs_true.dat records the generated model.xml.
• results_true.dat records k-effective and the tally sums: 37 short lines, so a plain diff is easier to read than a hashed result.
• _cleanup deletes the generated 2g.h5, as in mg_tallies.
• _compare_results adds assertions, as filter_energyfun does. These checks run before the reference comparison and use no stored values, so they test the scoring itself: a results_true.dat regenerated by the unfixed code would not make the test pass. On develop the test fails at the first of these checks.

Checking simulation output needs a transport run that banks delayed neutrons, and OpenMC keeps such tests in tests/regression_tests rather than in the unit tests.

Why the test needs no nuclear data. It runs in multigroup mode with a library it writes itself, so it needs neither OPENMC_CROSS_SECTIONS nor the NNDC library. The consistency checks compare tallies of the same events with each other, so they use no statistical tolerance and no analytic values derived from data. The reference files come from a build with -DOPENMC_ENABLE_STRICT_FP=on, as in CI, and the references of the existing MG tests reproduce on the same build. The test runs in about 0.2 s.

Before and after

"Before" is develop at a5bc348 and "after" is this PR's commit on top of it. All builds are RelWithDebInfo with strict FP and OpenMP on; the MPI build uses parallel HDF5. All runs use OMP_NUM_THREADS=2.

Build	History mode	Event mode (--event)	MPI (--mpi, 2 ranks)
a5bc348 (unfixed)	FAILED: the [DG, SL] check finds 2/4 bins mismatched, max relative difference 0.312	FAILED (same)	not run
this PR (fixed)	PASSED	PASSED	PASSED

Ratios of the tally means (history mode; event mode gives the same values to 1e-15):

Ratio	Bin	Before	After
Σ_EO [EO,DG,SL] / [DG,SL]	DG 1, P0	1.000000	1.000000
	DG 1, P1	0.700326	1.000000
	DG 2, P0	1.000000	1.000000
	DG 2, P1	0.687633	1.000000
Σ_DG [EO,DG,SL] / [EO,SL]	E_out < 0.625 eV, P1	0.701221	1.000000
	E_out > 0.625 eV, P1	0.687208	1.000000
max |ratio − 1|	all bins	0.3128	1.6e-15 (1.1e-15 with --mpi)

For example, the DG 1 P1 mean is 0.0394368 before and 0.0563121 after; the [DG, SL] reference is 0.0563121 in both runs. Before the fix the P1 bins scored the sum of (x/L)² instead of x/L. Between the results the test writes on develop and on this PR, only 8 numbers differ: the sum and sum of squares of the four P1 bins of the first tally. k-effective and the other two tallies are identical. The committed results_true.dat matches without regeneration.

Test suite

The test set of CI (tests/test_matplotlib_import.py tests/unit_tests tests/regression_tests) was run on a5bc348 and on this PR in the same environment: the NNDC HDF5 library (its cross_sections.xml matches the md5 in tests/conftest.py) and OMP_NUM_THREADS=2, in three pytest processes per run (the unit tests, and the regression tests split in two).

Mode	a5bc348	this PR
history	1494 passed, 147 skipped, 64 errors	1495 passed, 147 skipped, 64 errors
--event	1475 passed, 166 skipped, 64 errors	1476 passed, 166 skipped, 64 errors
--mpi --mpi-np=2 (2 ranks x 2 threads)	1414 passed, 157 skipped, 134 errors	1415 passed, 157 skipped, 134 errors

No test failed in any run. The one extra pass is the new test. The skipped tests and the tests with errors are the same on both sides, and every error is raised at setup by the environment:

• 61 tests need OPENMC_ENDF_DATA, which was not set (test_data_*.py, test_deplete_chain.py, test_endf.py).
• 3 tests (cpp_driver, source_dlopen, source_parameterized_dlopen) build against an installed OpenMC, and these builds were not installed.
• 70 tests, in the MPI run only, need mpi4py, which was not installed.

MPI run. The MPI run used Open MPI 4.1.6 with process binding off, through a small wrapper around mpiexec that unsets the OMPI_*, PMIX_*, OPAL_* and ORTE_* environment variables before it calls mpiexec. Without the wrapper, the in-process openmc.lib.init() of the CMFD regression tests (a singleton MPI_Init under Open MPI) leaves these variables in the pytest process, and every later mpiexec -n 2 openmc in that process aborts, on develop and on this PR alike.

Reference files. Regenerating every regression reference with pytest --update tests/regression_tests (history mode) on a5bc348 and on this PR gives byte-identical files, apart from run-to-run differences:

• At 2 threads, 58 of the 1137 files the two share differ, all in 8 directories (collision_track, the four deplete_* directories, microxs, surface_source, surface_source_write). All 58 also differ between two runs of develop.
• At 1 thread, 45 files differ, only in timestamps: the wall-clock depletion time dataset of the depletion results, and the modification times in HDF5 object headers. The same two kinds of fields differ between two 1-thread runs of develop.

No existing reference value changes.

Other checks.

• ctest on the serial and the MPI build: 14 tests, 13 passed, test_mcpl_stat_sum skipped because MCPL is not enabled. The same holds on a5bc348.
• clang-format 18.1.8 (--dry-run --Werror) reports no changes for src/tallies/tally_scoring.cpp. pyflakes 3.2.0 on the new test files and pycodestyle 2.11.1 on the Python diff report nothing.
• Environment: Ubuntu 24.04 (x86_64), GCC 13.3.0, HDF5 1.10.10 (serial, or Open MPI parallel for the MPI build), Python 3.12.3 with numpy 2.5.3, h5py 3.16.0, scipy 1.18.1, pandas 3.0.6 and pytest 9.1.1.

Not run (CI configurations not reproduced):

• MPICH: the MPI runs used Open MPI 4.1.6, through the wrapper above, without mpi4py.
• Builds with MCPL or coverage, and builds with OpenMP off.
• Builds with DAGMC, libMesh, XDG or NCrystal.
• The tests that need OPENMC_ENDF_DATA or an installed OpenMC (errors on both sides, listed above).
• CI's Python environment: the package versions above may differ from CI's.

Compatibility

• File formats: no change to input XML, statepoint or summary formats. The statepoint version is unchanged.
• API: no change to the Python API, the C API or openmc.lib.
• Results: results change only for the affected combination listed under "Motivation". There they become correct: weighted by w instead of w^2. Users who post-processed such tallies to undo the extra weight would need to drop that step.
• Existing tests: no reference results change (see "Reference files"). The only new reference files are those of the new test.
• Documentation: no change is needed. No documented behavior, input or interface changes, and the user's guide does not describe this scoring path.

Related PRs

• This PR has no dependencies; it is based on develop at a5bc348.
• A follow-up PR, "Add LifetimeMomentFilter for time moments since particle birth", is stacked on this one. Its regression test scores delayed-nu-fission with an EnergyoutFilter, a DelayedGroupFilter and the moment weights tau^n, which this bug would turn into tau^(2n). It should be merged after this PR.

AI Assistance

Code, tests, analysis, reviews and text:

• Harness: Claude Code
• Model: Claude Opus 5.5 (claude-opus-5-5)
• Reasoning effort: xhigh

Review of the pull request formalities:

• Harness: OpenCode
• Model: muse-spark
• Reasoning effort: not recorded

Review of the change:

• Harness: OpenCode
• Model: grok-4.7
• Reasoning effort: not recorded

Checklist

• I have performed a self-review of my own code
• I have run clang-format (version 18) 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)

[ondrejch]

The last CI run of this PR did not reach the code. In Tests and Coverage (run 37666792023, attempt 2), five jobs hung while installing packages, in apt update against the Azure Ubuntu mirror, and were cancelled at the six-hour job limit:

• CMake dependencies (fetched)
• Python 3.12 (omp=n, mpi=y, umesh_libs=, event=n)
• Python 3.12 (omp=n, mpi=n, umesh_libs=y, event=n)
• Python 3.12 (omp=y, mpi=n, umesh_libs=, event=n)
• Python 3.14t (omp=n, mpi=n, umesh_libs=, event=)

The C++ Format Check run was cancelled in the same way. The jobs that did run all passed, including Python 3.12 (omp=y, mpi=y). Could a maintainer re-run the failed jobs? Thank you.

[ondrejch]

Thanks for the re-run. Only Python 3.12 (omp=n, mpi=n, umesh_libs=y) was re-run, and it passed. These four jobs are still cancelled from the original apt hang, so Check CI status still fails:

• CMake dependencies (fetched)
• Python 3.12 (omp=n, mpi=y, umesh_libs=, event=n)
• Python 3.12 (omp=y, mpi=n, umesh_libs=, event=n)
• Python 3.14t (omp=n, mpi=n, umesh_libs=, event=)

Could they be re-run too?