Skip to content

Implement batch support - #15

Merged
Fuad-HH merged 3 commits into
mainfrom
multi-batch
Sep 14, 2026
Merged

Fuad-HH merged 3 commits into
mainfrom
multi-batch

Conversation

@Fuad-HH

@Fuad-HH Fuad-HH commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Add batch lifecycle management through AccumulateBatchTally and DiscardBatchTally methods. Track per-batch flux and compute mean and standard deviation across multiple batches for improved statistical accuracy. Refactor tally finalization to separate flux accumul
This pull request introduces batch-wise tally accumulation and statistical analysis for mesh tallies in PUMI-Tally. It adds the ability to accumulate and discard per-batch tallies, computes the mean and standard deviation of the flux over multiple batches, and updates the output to include these statistics. The changes also clarify the correct usage of initial position copying and batch handling in the documentation.

Batch tallying and statistics

  • Added AccumulateBatchTally and DiscardBatchTally methods to PumiTally and PumiTallyImpl to allow explicit closing or discarding of per-batch tallies, enabling correct computation of statistics over multiple batches. [1] [2] [3] [4] [5]
  • The ParticleAtElemBoundary struct now maintains running sums (flux_sum, flux_sum_sq) and a batch count (n_realizations) to compute the mean and standard deviation of the flux across batches. [1] [2]
  • Added ComputeMeanAndStdDev to calculate batch-wise statistics, and updated FinalizeTallies to output both mean and standard deviation, normalized by element volume. Output now includes both .osh and .vtu files. [1] [2]

Mesh tally output improvements

  • The tally output now writes the flux (mean) and flux_std_dev (standard deviation) tags, both normalized by element volume, in addition to the volume tag. The output behavior for single-batch runs is preserved for backward compatibility. [1] [2]

Documentation and usage clarification

  • Improved documentation for CopyInitialPosition to clarify that it must be called at the start of every batch, not just once per simulation, to ensure correct particle tracking.
  • Updated comments and assertions to reflect the new batch-wise workflow and prevent incorrect usage.

These changes provide more accurate and statistically meaningful mesh tally results, supporting workflows with multiple batches and inactive batches (e.g., for eigenvalue calculations).ation, statistics computation, and volume normalization into distinct functions. Output now includes both flux mean and standard deviation. Update file formats to .osh and _vtu.

Add batch lifecycle management through AccumulateBatchTally and
DiscardBatchTally methods. Track per-batch flux and compute mean and
standard deviation across multiple batches for improved statistical
accuracy. Refactor tally finalization to separate flux accumulation,
statistics computation, and volume normalization into distinct functions.
Output now includes both flux mean and standard deviation. Update file
formats to .osh and _vtu.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Correct the VTK output filename and update stale output-format documentation.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds batch lifecycle management and mean/standard-deviation flux statistics for mesh tallies.

Changes:

  • Adds batch accumulation and discard APIs.
  • Computes normalized batch statistics.
  • Produces .osh and VTK outputs.
File summaries
File Summary Review notes
src/pumitally/PumiTallyImpl.h Declares batch state and statistics. Nit (1 vote): document both output formats.
src/pumitally/PumiTallyImpl.cpp Implements batch processing, statistics, normalization, and output. Nits (2 votes, 1 vote): use an explicit .vtu filename and update stale output documentation.
src/pumitally/PumiTally.h Adds public batch APIs and documentation. Nit (2 votes): document both .osh and VTK outputs.
src/pumitally/PumiTally.cpp Implements public API wrappers and timing. No review comments.
Review details

Suppressed comments (2)

src/pumitally/PumiTallyImpl.cpp:169

  • Please update the user-facing output documentation along with this basename change. WriteTallyResults no longer produces the documented fluxresult.vtk; it now writes fluxresult.osh plus the VTK output using the _vtu basename, while README.md still says fluxresult.vtk and “VTK only”. Without updating those docs, users will look for a file that is no longer generated.
void PumiTallyImpl::WriteTallyResults() {
  p_pumi_particle_at_elem_boundary_handler->FinalizeTallies(full_mesh,
                                                            "fluxresult");

src/pumitally/PumiTallyImpl.h:148

  • This implementation now emits both Omega_h binary and VTK results, so describing finalization as writing only a VTK file is stale and makes the new .osh output easy to miss. Please update this internal/public implementation contract to name both output formats.
   * Normalize the accumulated flux with volume and write mean and standard
   * deviation in a VTK file
  • Files reviewed: 4/4 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/pumitally/PumiTally.h
Comment thread src/pumitally/PumiTallyImpl.cpp
@Fuad-HH
Fuad-HH merged commit 36d165d into main Sep 14, 2026
3 checks passed
@Fuad-HH
Fuad-HH deleted the multi-batch branch September 14, 2026 20:54
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