Skip to content

Vectorize _apply_impacts loop over realizations - #96

Merged
maritsandstad merged 2 commits into
basefrom
perf/vectorize-impacts-loop
Aug 3, 2026
Merged

Vectorize _apply_impacts loop over realizations#96
maritsandstad merged 2 commits into
basefrom
perf/vectorize-impacts-loop

Conversation

@benmsanderson

Copy link
Copy Markdown
Owner

Summary

MeteorInterface._apply_impacts was looping over realizations in Python:

for i in range(n_realizations):
    result = dd_model.calculate(temp_celsius[i])
    hdd_results.append(result.data["annual_hdd"].values)
    cdd_results.append(result.data["annual_cdd"].values)

The underlying DegreeDaysCalculator.calculate is already fully vectorized across the month dimension via xarray, so passing the entire (n_realizations, n_month) DataArray in one call produces identical results without the per-realization Python overhead.

Correctness

  • New test_degree_days_calculate_batches_realizations pins the contract: batched output and looped-per-realization output agree to atol=1e-9.
  • All existing 46 tests in tests/unit/impacts/ + tests/unit/test_meteor_interface.py still pass.

Wins

On the gen_impacts profiling workload at N=100 (via the sibling profiling harness):

  • DegreeDaysCalculator.calculate cumulative was 20.5 s of a 63 s workload — the serial loop was called 100 times per variable × 2 variables = 200 times.
  • After this change it's a rounding error in the profile.

Savings scale linearly in n_realizations.

Independence

Independent of the other perf/fix PRs in flight (#91#95). Touches _apply_impacts (around line 2089) — well away from any other in-flight edits to meteor_interface.py. Can land in any order.

Test plan

  • pytest tests/unit/impacts/test_degree_days.py::test_degree_days_calculate_batches_realizations
  • pytest tests/unit/impacts/ (full impacts test suite still passes)

🤖 Generated with Claude Code

benmsanderson and others added 2 commits July 19, 2026 15:42
MeteorInterface._apply_impacts was calling DegreeDaysCalculator.calculate()
inside a Python for-loop over realizations. The calculator itself is
already fully vectorized across the 'month' dimension via xarray, so
passing the entire (n_realizations, n_month) DataArray in one call
produces identical results without the per-realization Python overhead.

New test_degree_days_calculate_batches_realizations pins the contract
that batched output and per-realization output agree to atol=1e-9.

gen_impacts N=100 profile: DegreeDays.calculate cumulative was 20.5 s of
a 63 s workload; after this change it's a rounding error in the profile.
Wall-time savings scale linearly in n_realizations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- black reformats an assertion block in the new test
- After removing the per-realization loop, `n_realizations` from the
  shape unpack is now unused; use `n_months = ts_data.shape[1]` directly

Lint-only, no behavior change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@maritsandstad maritsandstad left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Fine this can probably just go in

@maritsandstad
maritsandstad merged commit f96b23a into base Aug 3, 2026
13 of 16 checks passed
@maritsandstad
maritsandstad deleted the perf/vectorize-impacts-loop branch August 3, 2026 18:04
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