Skip to content

Fix Regression: Historical Forcing Performance#107

Merged
mxkpp merged 3 commits intodevelopmentfrom
maxkipp-fix-historical-forcing-performance
Feb 23, 2026
Merged

Fix Regression: Historical Forcing Performance#107
mxkpp merged 3 commits intodevelopmentfrom
maxkipp-fix-historical-forcing-performance

Conversation

@mxkpp
Copy link

@mxkpp mxkpp commented Feb 23, 2026

This fixes a performance regression introduced in #79, specifically 84be3ed. As reviewer I had asked for @lru_cache to be removed from a class property to avoid the risk of one of the self attributes changing between calls, which would go unnoticed by lru_cache.

Discussion here: #79 (comment)

However, since method call causes a performance drop that scales exponentially with the duration of the simulation (the total number of timesteps, n). This is because each call to the method is O(n), and it is called N times, resulting in O(n^2).

It cannot be parameterized away from self usage, since one of its input is an unhashable type, dict.

To get the performance back to normal on Integration, I think this should be merged, but we should confirm soon that there are no cases in which the following are mutated after init, since that would cause the result stored in lru_cache to be invalid.

self.config_options.b_date_proc
self.config_options.fcst_input_horizons
self.config_options.fcst_freq

Alternatively, we could protect the above attributes by making them immutable after initial set.

Additions

  • Added log INFO calls for reporting timing of historical forcing cache fetch from s3.

Removals

Changes

  • Added @lru_cache to a O(n^2) method to fix performance issues

Testing

  1. I tested a calibration with -n 2 on a 4-core Workspace and got drastically better performance, back on par with Merge ngwpc-candidate into development for release 3.1.2.3.0-rc6 #89 which was the PR immediately prior to Add Alaska NWM Retrospective data to historical forcing engine #79.

Screenshots

Notes

Todos

  • Confirm or enforce that certain config_options attributes are never mutated during the run, since that would invalidate the value stored by lru_cache.

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows project standards (link if applicable)
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future todos are captured in comments
  • Visually tested in supported browsers and devices (see checklist below 👇)
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
  • Reviewers requested with the Reviewers tool ➡️

Testing checklist

Target Environment support

  • Linux

@mxkpp mxkpp merged commit b15d93e into development Feb 23, 2026
6 checks passed
@mxkpp mxkpp deleted the maxkipp-fix-historical-forcing-performance branch February 23, 2026 14:49
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