Fix Regression: Historical Forcing Performance#107
Merged
mxkpp merged 3 commits intodevelopmentfrom Feb 23, 2026
Merged
Conversation
mdeshotel
approved these changes
Feb 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes a performance regression introduced in #79, specifically 84be3ed. As reviewer I had asked for
@lru_cacheto be removed from a class property to avoid the risk of one of theselfattributes changing between calls, which would go unnoticed bylru_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
selfusage, 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_cacheto be invalid.Alternatively, we could protect the above attributes by making them immutable after initial set.
Additions
Removals
Changes
@lru_cacheto a O(n^2) method to fix performance issuesTesting
-n 2on 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
config_optionsattributes are never mutated during the run, since that would invalidate the value stored bylru_cache.Checklist
Testing checklist
Target Environment support