Skip to content

vectorized d1s#102

Open
jon-proximafusion wants to merge 2 commits into
developfrom
d1s-apply-time-correction-series
Open

vectorized d1s#102
jon-proximafusion wants to merge 2 commits into
developfrom
d1s-apply-time-correction-series

Conversation

@jon-proximafusion
Copy link
Copy Markdown
Collaborator

@jon-proximafusion jon-proximafusion commented May 19, 2026

Description

Adds apply_time_correction_series to openmc.deplete.d1s, a vectorized variant of apply_time_correction that evaluates many time indices in a single matrix multiplication.

Calling apply_time_correction in a loop over N time indices deep-copies the tally and re-multiplies its sum / sum_sq / mean / std_dev arrays N times. The new function builds an (N, n_radionuclides) factor matrix and folds the radionuclide-axis sum into a single matmul, so all indices are evaluated in one pass.

Returns NumPy arrays rather than a list of derived Tally objects: constructing N derived tallies (each with its own copy of _sum / _sum_sq / _mean / _std_dev) defeats the memory advantage on fine-mesh tallies. Users who need a Tally per index can build one from the returned arrays.

Motivation: shutdown dose-rate analysis routinely needs a full dose-vs-time curve, which means evaluating the same time_correction_factors dict at every index in the cooling schedule. For a 90-timestep schedule on a ~10⁶-voxel mesh the loop spends most of its time in repeated copy + elementwise multiply; the matmul-based path is ~5–15× faster on typical workloads (matmul hits BLAS, the per-iteration copy(tally) is gone, and _sum / _sum_sq are no longer materialized for results that are typically read-only).

Fixes # (issue) — N/A

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)

@jon-proximafusion
Copy link
Copy Markdown
Collaborator Author

three plausible shapes, each with a different tradeoff:

(a) Polymorphic index: keep one function, accept int | Sequence[int]. Return Tally for scalar, list[Tally] for sequence. Cleanest API surface, but the return type flips
with the input type — upstream maintainers often push back on this in review because static-type users and help() readers can't tell what they're getting.

(b) Add indices kwarg: keep index as-is, add a new indices=None parameter. If supplied, vectorized path runs and returns list[Tally] (or arrays). Lower review risk than
(a), but two parameters that do almost the same job is also a smell.

(c) Two functions (what I did): clearest discoverability, can return ndarray from the series path without violating any existing contract — but more API surface.

this pr does (c)

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