Skip to content

Fix nominal_resolution = "none" for realms with no dedicated model component - #633

Merged
rbeucher merged 2 commits into
mainfrom
fix_nominal_resolution_issue_in_CMIP7_CMORisation
Aug 24, 2026
Merged

Fix nominal_resolution = "none" for realms with no dedicated model component#633
rbeucher merged 2 commits into
mainfrom
fix_nominal_resolution_issue_in_CMIP7_CMORisation

Conversation

@rhaegar325

Copy link
Copy Markdown
Collaborator

Fix #632

Summary

_get_nominal_resolution() returned the source-CV value verbatim. Source CVs use
"none" to record that a model has no dedicated component for a realm — it is
not a resolution, and it is not a permitted nominal_resolution value in any CMIP
era. ACCESS has no land-ice model, so every landIce-realm variable wrote
nominal_resolution = "none" and failed [ATTR004].

This PR treats "none" as "no such component" and falls through to the realm that
actually produced the field.

Problem

[wcrp_cmip7:1.0 | weight 3] [ATTR004] Global attribute 'nominal_resolution' vocabulary check
    Invalid value(s) ['none'] for CV collection 'nominal_resolution'.

mrfso/snc/snw declare modeling_realm = "landIce land". The lookup took
realms[0] (landIce), found "none", and wrote it unchanged:

return self.source["model_component"][realm]["native_nominal_resolution"]

The value itself is factually correct — ACCESS-ESM1-6 has no ice-sheet component,
and PCMDI's own CMIP7 reference source entry writes "none" the same way. The bug
is that a source-registration field was used as the file-level
nominal_resolution attribute, which must be one of the 15 CV terms.

These fields are produced by CABLE on the atmosphere grid, so the correct value is
the land resolution, "250 km" — which is exactly what the published CMIP6
ACCESS-ESM1-5 LImon files carry.

Not CMIP7-only

The same defect exists in CMIP6Vocabulary, verified on an existing output file:

snw_LImon_ACCESS-ESM1-5_historical_r1i1p1f1_gn_020101-021012.nc
  nominal_resolution = 'none'

wcrp_cmip6:1.0 reports the identical [ATTR004] failure. It went unnoticed only
because compliance_check post-dates those runs. CMIP6 exposure is narrower —
mrfso sits in Lmon as "land landIce", so realms[0] is land and it passes
by luck. CMIP7 organises tables by realm, so all three variables break.

Fix

A shared module-level helper replaces the duplicated lookup tail in both classes:

resolution = registered(realm)
if resolution != "none":
    return resolution              # unchanged, including the None case

for fallback in realms:            # only "none" reaches here
    if fallback == realm:
        continue
    resolution = registered(fallback)
    if resolution not in (None, "none"):
        return resolution

raise ValueError(...)              # every realm registers "none"

Only the literal "none" triggers fall-through. A component that exists but omits
native_nominal_resolution still returns None, preserving current behaviour.

Realm selection, the multi-realm warning and target_realm validation are
untouched. CMIP6Vocabulary and CMIP7Vocabulary were line-for-line duplicates
here, which is why the bug existed in both; the helper removes that duplication.

Why raising is correct

The fall-through boundary lands exactly where it should:

CMIP7 table modeling_realm vars result
landIce landIce land 16 land = 250 km
landIce landIce 41 raises
atmosChem atmosChem aerosol 56 aerosol = 250 km
atmosChem atmosChem 21 raises

The landIce-only set is ice-sheet-specific (acabf, licalvf, topg,
sftgrf, xvelbase, …) — fields ACCESS cannot produce at all. A clear error is
the right answer there, not a borrowed 250 km.

Why the CV data is left alone

"none" is a true statement about the model; "250 km" under a landIce key
would assert an ice-sheet component that does not exist. Editing data also cannot
fix CMIP6: ACCESS-ESM1-5 and ACCESS-CM2 read from the upstream CMIP6_CVs git
submodule. The code-level fix covers all three vocabulary classes.

Scope

CMIP6Vocabulary, CMIP6PlusVocabulary (list-valued realms) and
CMIP7Vocabulary. No CV data, no submodules, no defaults changed.

@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.0%. Comparing base (dcb5336) to head (436ee36).

Additional details and impacted files
@@           Coverage Diff           @@
##            main    #633     +/-   ##
=======================================
+ Coverage   76.9%   77.0%   +0.1%     
=======================================
  Files         40      40             
  Lines       8509    8518      +9     
  Branches    1588    1593      +5     
=======================================
+ Hits        6547    6562     +15     
+ Misses      1629    1624      -5     
+ Partials     333     332      -1     
Flag Coverage Δ
unit 77.0% <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.

@rbeucher
rbeucher merged commit 590f0aa into main Aug 24, 2026
4 checks passed
@rbeucher
rbeucher deleted the fix_nominal_resolution_issue_in_CMIP7_CMORisation branch August 24, 2026 10:21
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.

Compliance_checker report: landIce variables write nominal_resolution = "none", which is not in the CMIP7 CV

2 participants