Mint esmvaltool CMIP6 regression baselines#760
Closed
lewisjared wants to merge 4 commits into
Closed
Conversation
…timeranges get_child_and_parent_dataset adds a timedelta branch offset to child_start, but catalogs loaded from YAML (e.g. test fixtures) carry start_time as a string rather than a datetime, raising "can only concatenate str (not datetime.timedelta) to str" for the ECS/TCR/TCRE/ZEC diagnostics. Parse the string back to a datetime before the arithmetic; only the year is used downstream so a stdlib datetime is sufficient. Constraint: replay/build never call this path, only execute does Confidence: high Scope-risk: narrow Not-tested: non-standard calendars with a string start_time
Add variable_id to the timeseries data-requirement group_by so production emits one execution per variable instead of a single combined execution, and trim the cmip6 test case to a single variable (tas) so the committed regression bundle is reviewable (series.json 11.5 MB -> 2.2 MB). Constraint: the test harness ignores group_by/constraints, so the per-variable split is exercised only in production; the test bundle shrink comes from the single-variable test case Rejected: shorten test time_span | recipe hardcodes start_year and CMIP6 files are not subset on fetch, so the series length is unaffected Rejected: reduce REGIONS | class-level, would also change production capture for annual-cycle Confidence: medium Scope-risk: moderate Not-tested: per-variable production solve (no test harness coverage for group_by)
Mint committed regression bundles, catalogs and manifests for the esmvaltool CMIP6 test cases (native blobs uploaded to the public baselines store). Disable the ozone-zonal CMIP7 test case, which has no working CMIP7 data yet. Confidence: high Scope-risk: narrow
|
Important Review skippedToo many files! This PR contains 251 files, which is 101 over the limit of 150. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (251)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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
1 participant
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.
Summary
Mints regression baselines for the esmvaltool CMIP6 test cases (native blobs uploaded to the public baselines store; committed bundles + manifests tracked here). Covers all 25 fetchable CMIP6 test cases across cloud, enso, ozone, regional-historical, sea-ice, climate-sensitivity (ECS/TCR/TCRE/ZEC) and timeseries diagnostics.
Two code changes were needed to get the climate-sensitivity and regional diagnostics minting cleanly:
fix(esmvaltool): stringstart_timein parent/child timerange alignmentget_child_and_parent_datasetadded atimedeltabranch offset tochild_start, but catalogs loaded from YAML (test fixtures) carrystart_timeas a string, raisingcan only concatenate str (not "datetime.timedelta") to strfor ECS/TCR/TCRE/ZEC. Now parsed back to a datetime first. Only the.yearis used downstream, so a stdlib datetime suffices. This path runs only during execute (not replay/build).refactor(esmvaltool): regional-historical-timeseries per variablevariable_idadded to the timeseriesgroup_byso each variable runs as its own execution.tas), shrinking the committedseries.jsonfrom 11.5 MB → 2.2 MB (the only oversized baseline).The time-span lever was investigated and rejected: the recipe hardcodes
start_yearand CMIP6 files aren't subset on fetch, sotime_spandoesn't change the series length. ReducingREGIONSwas rejected as it's class-level and would also change production capture for annual-cycle.Notes / follow-ups
chore(pmp): Mint all the baselinescommit that was already on this branch (not onmain). Rebase it out if you want an esmvaltool-only PR.ozone.py: the ozone-zonal CMIP7 test case is disabled (commented out) — no working CMIP7 data yet. Worth cleaning up (delete vs comment) before merge.'realm'KeyError or ESMValTool run errors) — expected, out of scope here.fetchis not resilient: an unhandled exception in one test case's catalog solve aborts the wholeref test-cases fetch --providerrun (this silently skipped 9 CMIP6 diagnostics until fetched per-diagnostic). Worth hardening separately./mnt/fressnapfmirror outage (input files temporarily gone). Their committed baselines + R2 native are valid, and the CI gate replays from R2 rather than re-executing.Test plan
ruff check,ruff format,mypypass (pre-commit)nativemanifest blocks