Skip to content

[BETTER SELF CONSISTENCY] Unique Tally object, closure and consistency reports - #83

Closed
SteSeg wants to merge 13 commits into
eepeterson:developfrom
SteSeg:no_more_pd_df_for_tallies
Closed

[BETTER SELF CONSISTENCY] Unique Tally object, closure and consistency reports#83
SteSeg wants to merge 13 commits into
eepeterson:developfrom
SteSeg:no_more_pd_df_for_tallies

Conversation

@SteSeg

@SteSeg SteSeg commented Apr 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR introduces a scalable, metadata-preserving tally workflow in OFB and aligns benchmark/UQ/TMC tally handling around a consistent API style.

It also addresses ambiguity in benchmark definitions (not only tallies), with a concrete fix for energy-spectrum closure semantics and improved consistency reporting.

Why These Changes Were Needed

1. Tally shape scalability

Before this PR, tallies were converted through a pandas.DataFrame step before becoming xarray objects.
That intermediate DataFrame representation is naturally tabular and effectively limits clean handling to 2D-like layouts, which becomes a bottleneck for multi-dimensional tallies.

This limitation affects both:

  • standard benchmark result workflows
  • UQ/TMC workflows, where tally structure and metadata must remain robust as complexity grows

2. Definition ambiguity across transport codes

We identified ambiguity in some benchmark definitions, especially energy-spectrum bin interpretation.

Example observed in Oktavian:

  • the same energy boundary list can be interpreted differently by OpenMC and MCNP
  • OpenMC treats the first value as the lowest energy bound
  • MCNP may implicitly prepend a lower bound (code/user cutoff), shifting the effective binning

This can produce shifted/discordant results from nominally "same" inputs.
To address this ambiguity, we implemented the concept of closure in benchmark specifications.yaml file, which uniquely defines the correct interpretation of a given array of bins for tallies.

Main Changes In This PR

  • Added a new backends/ folder for transport-code specific implementations.
  • Current backend coverage: OpenMC.
  • Implemented explicit energy-spectrum closure semantics in specifications and handling.
  • Added tally consistency checks and user-facing consistency reports.
  • Added a new OFB Tally object in BenchmarkResults:
    • no pandas intermediary
    • manipulation style closer to OpenMC tally usage
  • Harmonized tally objects/interfaces across:
    • BenchmarkResults (including UQ runs)
    • TMCStatePoint / TMCTally
    • (deeper harmonization details will continue in future PRs)
  • Added dimension-report capabilities so users can quickly inspect tally shape/axes.
  • Added a specifications consistency report that allows to check whether tally resulting from a benchmark run are consistent with what expected in the specifications file

Minimal Usage Examples

1. Energy closure in specification

# example snippet from a spectrum definition
energy:
  bins: [1e-9, 1e-8, 1e-7, 1e-6]
  closure: "[low, high)"

2. Read benchmark results and work with OFB Tally object

from openmc_fusion_benchmarks import BenchmarkResults

br = BenchmarkResults("benchmark_results.h5")

# OFB-style tally access (no pandas conversion required)
tally = br.get_tally("reaction_rate")
print(tally.dimension_report())          # quick shape/axis inspection
print(tally.format_dimension_report())   # readable report

3. Call consistency report

from openmc_fusion_benchmarks import BenchmarkResults

br = BenchmarkResults("benchmark_results.h5")
print(br.format_spec_consistency_report())

@eepeterson

Copy link
Copy Markdown
Owner

Generally this seems a bit big to be a single PR. Can the functionality be broken out into slightly smaller chunks? Also, I don't love the addition of the closure in the benchmark spec. Because we control the spec, we can decide what it means. I would advocate that explicit is better than implicit and therefore interpret bins as the bin edges with no implicit additional bins either above or below so that for n bin edges you get n-1 bins. We could even change the name from bins to values or 'bin_edges to be parallel to how OpenMC works. How that gets translated into MCNP tally functionality is not our problem right now since we aren't developing the back end for MCNP.

If we are taking the MCNP input to be the definition of the spec then we can add a bin below the lowest tally bin edge at the cutoff value to the spec.

@SteSeg

SteSeg commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator Author

Generally this seems a bit big to be a single PR. Can the functionality be broken out into slightly smaller chunks? Also, I don't love the addition of the closure in the benchmark spec. Because we control the spec, we can decide what it means. I would advocate that explicit is better than implicit and therefore interpret bins as the bin edges with no implicit additional bins either above or below so that for n bin edges you get n-1 bins. We could even change the name from bins to values or 'bin_edges to be parallel to how OpenMC works. How that gets translated into MCNP tally functionality is not our problem right now since we aren't developing the back end for MCNP.

If we are taking the MCNP input to be the definition of the spec then we can add a bin below the lowest tally bin edge at the cutoff value to the spec.

Of course, i can split the PR in multiple ones and remove the closure object, probably going for bin_edges then. I'll open one PR for the Tally object and one for the consistency reports.

@SteSeg SteSeg closed this Aug 14, 2026
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.

2 participants