Repository navigation
Fix zostoga recomputing its reference fields for every split, and stop it resuming mid-run - #746
Open
rhaegar325 wants to merge 2 commits into
Open
rhaegar325 wants to merge 2 commits into
rhaegar325 wants to merge 2 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #746 +/- ##
=====================================
Coverage 79.7% 79.8%
=====================================
Files 41 41
Lines 9173 9173
Branches 1710 1710
=====================================
+ Hits 7315 7317 +2
+ Misses 1524 1523 -1
+ Partials 334 333 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Problem
calc_zostogakept its reference fields (temp_ref, anddzt_refas a time-mean over the whole run) as lazy graphs. Each output split is computed with its ownclient.compute(), so every split re-read and re-averaged the fulldztrecord. Cost grew quadratically with run length.--resumethe worker drops input files before the checkpoint year. Forzostogathis changes the reference state, so the series restarts from zero at the resume point and the new files sit in the same version directory as the earlier ones, with no error raised. This becomes reachable once Fix--resumefor CMIP7 compound variable names #743 makes CMIP7 resume work.Changes
calc_zostoga: persisttemp_refand the time-mean ofdzt_refonce. Both are single (depth, lat, lon) fields (~21.6 MB in float32). The result stays dask-backed, and output values are unchanged.zostogawith"resumable": false.resumable: false, ignore the resume checkpoint and reprocess the variable in full.Scope of the resume change
I audited every formula operation in the mappings.
calc_zostogais the only one whose value at a given time depends on input from other times. Everything else is pointwise in time, a monthly resample (resume always restarts on a year boundary), or a fixed field. So onlyzostogais opted out, and all other variables keep resume.Testing
dztinput; this test fails if the persist is removed;zostogais markedresumable: falsein both mappings;client.compute()calls under a distributed client gives output identical to the previous code (max difference 0.0).