feat(ilamb): use obs4REF reference data for standard diagnostics#779
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR updates obs4REF handling end to end: parsing and validation now accept the newer reference keys, ILAMB configuration and execution now resolve obs4REF-backed sources, and the affected regression fixtures, manifests, and catalogues were refreshed for the migrated diagnostics. Changesobs4REF reference dataset migration
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Point ten ILAMB standard diagnostics at their obs4REF reference files instead of the legacy ilamb/iomb registry paths: gpp-WECANN, gpp-FLUXNET2015, mrro-LORA, cSoil-HWSD2, nbp-Hoffman, snc-ESACCI and burntFractionAll-GFED on land, plus thetao/so-WOA2023-surface and amoc-RAPID in the ocean suite. burntFractionAll-GFED now reads the CMIP burntFractionAll variable directly rather than the legacy burntArea alternate, and amoc-RAPID derives the AMOC index from the RAPID-2023-1a msftmz field through the existing msftmz_to_rapid transform. Bumps ILAMBStandard.version to 3: the reference change alters diagnostic outputs, so the ILAMB regression baselines must be re-minted.
…er diagnostic Replace the bare obs4REF registry paths with the ingested obs4MIPs data requirement form, pinning each reference to a single file by source_id + variable_id + grid_label + version. This routes the reference through the obs4MIPs catalogue as a first-class dataset rather than a raw registry key. Loosen the obs4REF key parser to accept monthly-climatology (monC) frequencies and single-year fixed-field (fx) time ranges so WOA-23 and HWSD-2-0 files resolve. Give each rewired diagnostic its own version (3) via a per-diagnostic ``version`` config key, so changing one diagnostic's reference data no longer forces every ILAMB baseline to be re-minted; the untouched diagnostics stay at version 2. The esgf-catalog test fixture and solve-regression snapshots are regenerated to include the newly ingested obs4REF reference datasets.
obs4REF is the REF-specific observational product and follows the same metadata conventions as obs4MIPs, but its files carry ``activity_id = "obs4REF"``. The obs4MIPs adapter rejected them, so obs4REF references could not be ingested and any diagnostic that requests an obs4REF dataset as an obs4MIPs source solved to nothing. Accept ``activity_id`` of either ``obs4MIPs`` or ``obs4REF`` so these references ingest as first-class datasets.
…urce_id The FLUXNET2015 obs4REF file spells its source_id three different ways across the registry path (FLUXNET2015-1-0), the filename (Fluxnet-2015-1-0) and the internal attribute (Fluxnet-2015), so the ingested facet form cannot match both the fetch and solve stages. Reference it by exact registry key until the upstream metadata is reconciled.
…egistry data ILAMBStandard.execute() replaced the reference dataframe with only the ingested obs4MIPs datasets, dropping the ILAMB registry collection. A diagnostic combining an obs4REF primary source with legacy-path relationships (e.g. gpp-WECANN's pr/tas) could no longer resolve those relationship keys. Concatenate the ILAMB and obs4REF registry collections alongside the ingested obs4MIPs datasets so those keys stay resolvable. Re-mint obs4REF baselines for the affected land and ocean diagnostics: burntfractionall-gfed, csoil-hwsd2, mrro-lora, nbp-hoffman, snc-esacci, so-woa2023-surface and thetao-woa2023-surface (cmip6 + cmip7), plus cmip6 for gpp-wecann and gpp-fluxnet2015. The cmip7 baselines for gpp-wecann and gpp-fluxnet2015 remain outstanding pending an upstream ilamb3 fix for a ZeroDivisionError on the CMIP7 path; amoc-rapid likewise stays failing.
lewisjared
force-pushed
the
feat/ilamb-obs4ref-references
branch
from
21265c3 to
ca7986d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…apshots Bump test_case_version for the re-minted ILAMB baselines so the coupling gate authorises the changed bundles, and refresh the solver regression snapshots to match the obs4REF-referenced data requirements after rebasing onto main.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fa3ee72e-2fea-4081-8ca9-7e4d81b7c1ca
📒 Files selected for processing (25)
packages/climate-ref-ilamb/tests/test-data/burntfractionall-gfed/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/burntfractionall-gfed/cmip7/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/csoil-hwsd2/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/csoil-hwsd2/cmip7/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/gpp-fluxnet2015/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/gpp-wecann/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/mrro-lora/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/mrro-lora/cmip7/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/nbp-hoffman/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/nbp-hoffman/cmip7/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/snc-esacci/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/snc-esacci/cmip7/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/so-woa2023-surface/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/so-woa2023-surface/cmip7/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/thetao-woa2023-surface/cmip6/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/thetao-woa2023-surface/cmip7/manifest.jsonpackages/climate-ref-ilamb/tests/unit/test_solve_regression/test_solve_regression_amoc_rapid_.ymlpackages/climate-ref-ilamb/tests/unit/test_solve_regression/test_solve_regression_burntfractionall_gfed_.ymlpackages/climate-ref-ilamb/tests/unit/test_solve_regression/test_solve_regression_csoil_hwsd2_.ymlpackages/climate-ref-ilamb/tests/unit/test_solve_regression/test_solve_regression_gpp_wecann_.ymlpackages/climate-ref-ilamb/tests/unit/test_solve_regression/test_solve_regression_mrro_lora_.ymlpackages/climate-ref-ilamb/tests/unit/test_solve_regression/test_solve_regression_nbp_hoffman_.ymlpackages/climate-ref-ilamb/tests/unit/test_solve_regression/test_solve_regression_snc_esacci_.ymlpackages/climate-ref-ilamb/tests/unit/test_solve_regression/test_solve_regression_so_woa2023_surface_.ymlpackages/climate-ref-ilamb/tests/unit/test_solve_regression/test_solve_regression_thetao_woa2023_surface_.yml
✅ Files skipped from review due to trivial changes (10)
- packages/climate-ref-ilamb/tests/test-data/nbp-hoffman/cmip7/manifest.json
- packages/climate-ref-ilamb/tests/test-data/burntfractionall-gfed/cmip6/manifest.json
- packages/climate-ref-ilamb/tests/test-data/snc-esacci/cmip6/manifest.json
- packages/climate-ref-ilamb/tests/test-data/csoil-hwsd2/cmip6/manifest.json
- packages/climate-ref-ilamb/tests/test-data/so-woa2023-surface/cmip6/manifest.json
- packages/climate-ref-ilamb/tests/test-data/gpp-wecann/cmip6/manifest.json
- packages/climate-ref-ilamb/tests/test-data/thetao-woa2023-surface/cmip6/manifest.json
- packages/climate-ref-ilamb/tests/test-data/snc-esacci/cmip7/manifest.json
- packages/climate-ref-ilamb/tests/test-data/thetao-woa2023-surface/cmip7/manifest.json
- packages/climate-ref-ilamb/tests/test-data/csoil-hwsd2/cmip7/manifest.json
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/climate-ref-ilamb/tests/test-data/mrro-lora/cmip7/manifest.json
- packages/climate-ref-ilamb/tests/test-data/burntfractionall-gfed/cmip7/manifest.json
- packages/climate-ref-ilamb/tests/test-data/so-woa2023-surface/cmip7/manifest.json
- packages/climate-ref-ilamb/tests/test-data/mrro-lora/cmip6/manifest.json
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
The obs4REF diagnostics require an obs4MIPs reference dataset to solve, but
the solve-regression catalog fixture contained none of them, so those
diagnostics matched nothing and their snapshots regenerated to `{}` -
passing while asserting no solver output.
Append the obs4REF reference datasets to obs4mips_catalog.parquet (221 -> 231
rows, existing rows untouched) and regenerate the affected snapshots so each
diagnostic again asserts its matched CMIP groups. Only emp-GLEAMGPCP2.3
remains empty, matching its GLEAM reference that is intentionally absent from
the fixture.
An empty snapshot passes while asserting no solver output, silently hiding a diagnostic that matches nothing because its reference data is missing from the solve catalog fixture. Assert each diagnostic solves to at least one execution group unless it is listed in KNOWN_NO_MATCH (currently only emp-GLEAMGPCP2.3, whose GLEAM reference is intentionally absent).
The converter applied least_significant_digit=3 (~1e-3 absolute precision) to every variable. gpp magnitudes (~1e-8 kg m-2 s-1) fall five orders of magnitude below that floor, so the entire converted field rounded to zero. A spatially uniform (zero-variance) field made ilamb3's Taylor spatial-distribution score divide by 1/norm_std == 0, so gpp-wecann and gpp-fluxnet2015 failed on the CMIP7 path. Skip the decimal-place quantization for gpp; other variables are unaffected and their converted output is byte-identical. Adds the now-passing CMIP7 obs4REF baselines for gpp-wecann and gpp-fluxnet2015.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/climate-ref-core/src/climate_ref_core/esgf/cmip7.py (1)
99-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded
gppexception is fragile for future variables.The fix correctly resolves the immediate Taylor-score regression, but hardcoding a single variable name means any other future variable with similarly tiny magnitudes (e.g. other carbon/nitrogen fluxes) would silently be rounded to zero again, and only be caught if a downstream score visibly breaks.
Consider a more general safeguard, e.g. skip
least_significant_digitwhen the variable's data magnitude is below some threshold relative to the requested precision, or maintain an explicit, documented allow-list of "lossless" variables that's easy to extend.♻️ Example: magnitude-based safeguard
encoding: dict[str, dict[str, Any]] = {} for var in ds_cmip7.data_vars: var_encoding: dict[str, Any] = {"zlib": True, "complevel": 5} - if str(var) != "gpp": + data = ds_cmip7[var].values + max_abs = float(abs(data).max()) if data.size else 0.0 + # Skip quantization if it would round the entire field to zero. + if max_abs == 0.0 or max_abs >= 10 ** -3: var_encoding["least_significant_digit"] = 3 encoding[str(var)] = var_encoding
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 57f131fc-c430-4580-8f1a-0e2c5a69e879
📒 Files selected for processing (11)
packages/climate-ref-core/src/climate_ref_core/esgf/cmip7.pypackages/climate-ref-ilamb/tests/test-data/gpp-fluxnet2015/cmip7/catalog.yamlpackages/climate-ref-ilamb/tests/test-data/gpp-fluxnet2015/cmip7/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/gpp-fluxnet2015/cmip7/regression/diagnostic.jsonpackages/climate-ref-ilamb/tests/test-data/gpp-fluxnet2015/cmip7/regression/output.jsonpackages/climate-ref-ilamb/tests/test-data/gpp-fluxnet2015/cmip7/regression/series.jsonpackages/climate-ref-ilamb/tests/test-data/gpp-wecann/cmip7/catalog.yamlpackages/climate-ref-ilamb/tests/test-data/gpp-wecann/cmip7/manifest.jsonpackages/climate-ref-ilamb/tests/test-data/gpp-wecann/cmip7/regression/diagnostic.jsonpackages/climate-ref-ilamb/tests/test-data/gpp-wecann/cmip7/regression/output.jsonpackages/climate-ref-ilamb/tests/test-data/gpp-wecann/cmip7/regression/series.json
✅ Files skipped from review due to trivial changes (6)
- packages/climate-ref-ilamb/tests/test-data/gpp-fluxnet2015/cmip7/regression/diagnostic.json
- packages/climate-ref-ilamb/tests/test-data/gpp-wecann/cmip7/manifest.json
- packages/climate-ref-ilamb/tests/test-data/gpp-fluxnet2015/cmip7/manifest.json
- packages/climate-ref-ilamb/tests/test-data/gpp-fluxnet2015/cmip7/regression/output.json
- packages/climate-ref-ilamb/tests/test-data/gpp-fluxnet2015/cmip7/regression/series.json
- packages/climate-ref-ilamb/tests/test-data/gpp-wecann/cmip7/regression/series.json
…registry Wire lai-AVH15C1 to the obs4MIPs NOAA-NCEI-LAI-AVHRR-5-0 reference (dict-form source, version 3) and re-mint its cmip6/cmip7 baselines. The reference is ~0.05 degrees, far finer than any CMIP model, so add a climate_ref_coarsen_spatial transform that conservatively coarsens very-fine references to 0.5 degrees before comparison; coarser fields (models) pass through unchanged. Prune the registry entries now provided by obs4REF/obs4MIPs from ilamb.txt and repoint the string-form example in test_standard_metrics to lai. Add synthetic evspsbl rows to the CMIP6/CMIP7 solve fixtures so emp-GLEAMGPCP2.3 solves against real model data instead of an empty snapshot, and add the lai obs4MIPs rows to the obs4mips solve fixture.
…ferences * origin/main: fix(core): validate end_month and clamp day in shift_time_axis_end test: extend ENSO CMIP7 recipe timeranges for fabricated 2021 coverage docs: add changelog entry for CMIP7 historical coverage fabrication feat(core): fabricate extended CMIP7 historical coverage for tests chore: clean up docs chore: changelog ci: don't include descriptions of the production deployment docs: add changelog entry for ESMValTool CI memory limits ci: bound ESMValTool memory use in integration tests refactor(models): reusable latest-execution ranking with pre/post-rank success filters # Conflicts: # tests/test-data/esgf-catalog/cmip7_catalog.parquet
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
Points eleven ILAMB standard diagnostics at published reference data ingested through the REF catalog: ten at obs4REF and one (
lai-AVH15C1) at obs4MIPs, replacing the legacy bundledilamb/iombregistry files. The underlying obs4REF data was registered and uploaded in #778, now merged, so this PR is standalone.How references are resolved
References are now given as structured, ingested facets rather than bundled file paths. The obs4MIPs adapter (
parse_obs4mips) is extended to acceptactivity_id: obs4REFalongsideobs4MIPs, so obs4REF products flow through the same ingestion path and become solve-visible datasets. Most diagnostics therefore use theobs_source: obs4refdict form keyed onsource_id/variable_id/grid_label/version, resolved against the ingested catalog at solve time.Two parser limitations that previously blocked the dict form are lifted in
_parse_obs4ref_key: the frequency token now accepts a trailing capital (monCclimatologies, e.g. WOA-23) and the time-range token now accepts a single year rather than only a start-end pair (fxfields, e.g. HWSDcSoil). This is what lets WOA-23 and HWSD move to the ingested form.Diagnostics rewired
Land (
ilamb.yaml):gpp-WECANN(WECANN-1-0),mrro-LORA(LORA-1-0),cSoil-HWSD2(HWSD-2-0),nbp-Hoffman(Hoffman-1-0),snc-ESACCI(CCI-CryoClim-FSC-1),burntFractionAll-GFED(GFED-5-0) — all via ingested obs4REF facets.gpp-FLUXNET2015points at obs4REF but by exact registry key rather than facets, because that file'ssource_idis spelled three different ways (#784).lai-AVH15C1moves to obs4MIPsNOAA-NCEI-LAI-AVHRR-5-0.Ocean (
iomb.yaml):thetao-WOA2023-surfaceandso-WOA2023-surface(WOA-23 monthly climatology, surface viaselect_depth),amoc-RAPID(RAPID-2023-1a).Edits beyond a source swap
lai-AVH15C1: the AVHRR reference is ~0.05 deg (5 km), far finer than any CMIP model, which makes ilamb3's regrid/scoring step intractable. A new_CoarsenSpatialILAMB transform conservatively coarsens fields finer than a 0.5 deg target (i.e. the reference) and leaves model-resolution fields untouched.burntFractionAll-GFED: now reads the CMIPburntFractionAllvariable directly and drops theburntAreaalternate, because the obs4REF file carriesburntFractionAll(gridgr).amoc-RAPID: switches its reference to the raw RAPID-2023-1amsftmzfield and relies on the existingmsftmz_to_rapidtransform to derive the AMOC index, replacing the pre-derived legacyamocfile.WOA-23(thetao/so): the ingested reference is a 2005-2022 monthly climatology (monC), replacing the legacy 2005-2014 monthly field; the old "starts beyond historical, cannot use for CMIP6" caveat no longer applies.Mixed obs4REF + registry resolution: when a diagnostic's sources are ingested, its relationship analyses (and any remaining string-path sources) still refer to keys in the ILAMB/obs4REF registries.
execute()now concats the ingested obs4MIPs datasets with the ILAMB and obs4REF registry collections so those keys stay resolvable.gpplossless conversion (esgf/cmip7.py): CMIP6->CMIP7 conversion appliedleast_significant_digit=3to every variable, but gpp magnitudes (~1e-8 kg m-2 s-1) sit far below that ~1e-3 precision floor and rounded the whole field to zero. gpp is now kept lossless; other variables keep the lossy encoding.Versioning and baselines
Rather than bumping the shared
ILAMBStandard.version, each rewired diagnostic now carries its ownversion: 3override (the class default stays at 2). Only the eleven rewired diagnostics are treated as stale, so only their regression baselines need re-minting — the untouched ILAMB diagnostics keep their existing baselines and the coupling gate stays scoped.The pruned legacy files (
cSoil,gppWECANN/FLUXNET,lai,mrro,nbp,snc,burntFractionAll) are removed fromdataset_registry/ilamb.txt.Validation
ILAMB unit suite passes; ruff/mypy/pre-commit clean; core, integration, and provider CI jobs green. Regression fixtures and solve snapshots have been regenerated. The
coupling-gatejob is currently red because the eleven re-minted baselines still need to be authorised on the native store; that is the expected pre-mint state for this change.Not yet fully validated end-to-end against the real reference data.
amoc-RAPIDin particular is unverified: it carries both themsftmz_to_rapidtransform-semantics question above and the known upstream ilamb3trim_timecrash, and needs a real run to confirm.Summary by CodeRabbit
New Features
Bug Fixes
Chores / Configuration
Tests