Skip to content

Drop dangling grid_mapping (and um_stash_source) from atmosphere CMOR output#468

Merged
rbeucher merged 1 commit into
mainfrom
remove-remaining-attrs-from-rawdata
Jun 30, 2026
Merged

Drop dangling grid_mapping (and um_stash_source) from atmosphere CMOR output#468
rbeucher merged 1 commit into
mainfrom
remove-remaining-attrs-from-rawdata

Conversation

@rhaegar325

Copy link
Copy Markdown
Collaborator

Summary

CMORised ACCESS-ESM1-5 atmosphere (Amon) files failed the CF check:

§5.6 grid mapping variable latitude_longitude must exist in this dataset

The fix drops the model-native grid_mapping attribute (and the related
um_stash_source provenance attribute) that leak from the raw UM variable into the
CMORised output. Affected all 9 Amon variables in the test set.

Root cause

  1. The raw UM atmosphere files put grid_mapping = "latitude_longitude" on each source
    variable and include a latitude_longitude grid-mapping container variable
    (valid CF).
  2. The atmosphere CMORiser's direct/formula path renames the source variable to the CMOR
    name (self.ds.rename({raw_var: cmor_name})), which keeps the source variable's
    attributes
    . update_attributes then merges the CMOR-table attributes on top with
    attrs.update(...), without clearing the inherited ones.
  3. The latitude_longitude container variable is not carried into the CMORised output
    (only the target variable + coordinates/bounds are kept).

Net result: the output variable keeps grid_mapping = "latitude_longitude" pointing at a
variable that no longer exists → a dangling reference that fails the CF grid-mapping check.
The same mechanism leaks um_stash_source (a UM STASH code).

This was confirmed against the published reference: official CMIP6 Amon/tas has neither
grid_mapping nor a latitude_longitude variable — regular lat-lon grids carry no grid
mapping.

Change

src/access_moppy/atmosphere.py — in update_attributes, alongside the existing
attrs.pop("axis") / attrs.pop("positive") cleanup:

# Drop model-native attributes inherited from the source variable via the
# rename above that have no place in CMIP6 output:
#  - grid_mapping: a regular lat-lon CMIP6 grid carries none, and its
#    container variable is not carried into the output, so the attribute
#    is a dangling reference that fails the CF grid-mapping check.
#  - um_stash_source: a UM STASH provenance code, absent from the
#    published reference.
self.ds[self.cmor_name].attrs.pop("grid_mapping", None)
self.ds[self.cmor_name].attrs.pop("um_stash_source", None)

Tests

tests/unit/test_atmosphere.pyTestUpdateAttributesDecodedTime::test_model_native_attributes_dropped
sets grid_mapping and um_stash_source on the variable, runs update_attributes, and
asserts both are gone.

tests/unit/test_atmosphere.py: 56 passed.

@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.2%. Comparing base (09b4eb1) to head (d00c4ea).

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #468   +/-   ##
=====================================
  Coverage   77.2%   77.2%           
=====================================
  Files         31      31           
  Lines       6030    6032    +2     
  Branches    1122    1122           
=====================================
+ Hits        4653    4655    +2     
  Misses      1126    1126           
  Partials     251     251           
Flag Coverage Δ
unit 77.2% <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 488131e into main Jun 30, 2026
4 checks passed
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.

2 participants