Skip to content

Fix zostoga recomputing its reference fields for every split, and stop it resuming mid-run - #746

Open
rhaegar325 wants to merge 2 commits into
mainfrom
fix_zostoga
Open

rhaegar325 wants to merge 2 commits into
mainfrom
fix_zostoga

Conversation

@rhaegar325

Copy link
Copy Markdown
Collaborator

Problem

  • Efficiency: calc_zostoga kept its reference fields (temp_ref, and dzt_ref as a time-mean over the whole run) as lazy graphs. Each output split is computed with its own client.compute(), so every split re-read and re-averaged the full dzt record. Cost grew quadratically with run length.
  • Resume correctness: on --resume the worker drops input files before the checkpoint year. For zostoga this 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 --resume for CMIP7 compound variable names #743 makes CMIP7 resume work.

Changes

  • calc_zostoga: persist temp_ref and the time-mean of dzt_ref once. Both are single (depth, lat, lon) fields (~21.6 MB in float32). The result stays dask-backed, and output values are unchanged.
  • Mappings (ACCESS-ESM1-5, ACCESS-ESM1-6): mark zostoga with "resumable": false.
  • Worker template: when the mapping has 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_zostoga is 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 only zostoga is opted out, and all other variables keep resume.

Testing

  • New unit tests:
    • the result graph no longer contains the dzt input; this test fails if the persist is removed;
    • zostoga is marked resumable: false in both mappings;
    • the rendered worker template contains the gate.
  • Writing two splits with separate client.compute() calls under a distributed client gives output identical to the previous code (max difference 0.0).
  • Full unit suite: 2255 passed.
  • Not yet run on real data.

@codecov

codecov Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.8%. Comparing base (a78c3fc) to head (00053be).

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     
Flag Coverage Δ
unit 79.8% <100.0%> (+<0.1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rhaegar325
rhaegar325 requested a review from rbeucher October 9, 2026 05: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.

1 participant