Feat/cmip7 awiesm3 veg hr - #266
Open
JanStreffing wants to merge 395 commits into
Open
Conversation
JanStreffing
requested review from
christian-stepanek,
mandresm,
nwieters,
pgierz and
siligam
April 2, 2026 12:55
JanStreffing
force-pushed
the
feat/cmip7-awiesm3-veg-hr
branch
from
April 2, 2026 13:18
1a42875 to
5617a18
Compare
Contributor
Author
Contributor
Author
|
The single failing test ( Something to worry about? @pgierz |
Contributor
|
@pgierz, will you review this or should I go for it? |
Member
|
I will look |
JanStreffing
commented
Apr 22, 2026
| return ds | ||
|
|
||
|
|
||
| def _attach_bounds_from_mesh(ds, rule, coord_names): |
Contributor
Author
There was a problem hiding this comment.
Check if generic enough for backend
JanStreffing
commented
Apr 22, 2026
| return any(v.chunks is not None for v in ds.data_vars.values()) | ||
|
|
||
|
|
||
| def _encoding_from_dask_chunks(ds, rule): |
Contributor
Author
There was a problem hiding this comment.
Deferred for later review
JanStreffing
commented
Apr 22, 2026
|
|
||
| if table_id is None: | ||
| # Fallback to user-provided | ||
| table_id = self.rule_dict.get("table_id", None) |
Contributor
Author
There was a problem hiding this comment.
Should be checked. Why deleted?
JanStreffing
commented
Apr 22, 2026
| """ | ||
| return self.rule_dict.get("Conventions", "CF-1.11") | ||
|
|
||
| # ======================================================================== |
Contributor
Author
There was a problem hiding this comment.
Should be a class. Do we need it?
JanStreffing
commented
Apr 22, 2026
| return "hdl:21.14100/" + str(uuid.uuid4()) | ||
| """Generate a unique tracking ID (prefix overridable via rule_dict). | ||
|
|
||
| The CMIP7 tracking_id CV requires the ``hdl:21.14107/<uuid>`` prefix |
Contributor
Author
There was a problem hiding this comment.
Does that mean cmip6 wont work anymore?
JanStreffing
commented
Apr 22, 2026
| approx_interval = drv.table_header.approx_interval | ||
| frequency_str = _frequency_from_approx_interval(approx_interval) | ||
| logger.debug(f"{approx_interval=} {frequency_str=}") | ||
| # attach the frequency_str to rule, it is referenced when creating file name | ||
| rule.frequency_str = frequency_str | ||
| time_method = _get_time_method(drv.frequency) | ||
| rule.time_method = time_method | ||
| # FESOM yearly files and concat'd hemispheric selects can yield a |
Contributor
Author
There was a problem hiding this comment.
Hallucination? xarray can sort itself?
wcrp FILE004d (HIGH) requires each data-variable chunk to be at least 4 MiB uncompressed (CMIP7 storage convention). The dask- aligned path in ``_encoding_from_dask_chunks`` mirrors whatever dask chunks the pipeline produced; on FESOM unstructured HR fields (sfx: 12 × 3146761 nodes, dask chunks (12, 18724)) that lands at 1.8 MiB per netCDF chunk and trips the check. cmip7repack is the documented post-hoc fix but for these 2D high-res cases it re-uses the same shape and the file stays under threshold. Add a per-variable floor: if dask-aligned chunks are below 4 MiB, grow the trailing (rightmost, typically horizontal) dim's chunk until product × wordsize crosses the threshold, capped at the dim size. Leaves the time-axis chunking pycmor already picked alone so streaming writes stay aligned with dask. The non-dask path (``calculate_chunks_simple``) already targets 100 MB so it doesn't need the floor. Clears 4× FILE004d HIGH (sfx, somint, phcint, absscint) on the cli71 production batch.
Two cli72 residual HIGH fixes whose root causes were in pycmor:
- time_bounds: extend the period-start pin (previously yearly-only)
to every rule wcrp_cmip7 TIME001 treats as instantaneous. wcrp's
use_midpoint=False fires for (a) any rule with cell_methods
"time: point" OR (b) any frequency NOT in its AVG_CORRECTION_FREQ
set {day, mon, monPt, yr, yrPt, 1hrCM, sem}. So monthly tpt
(sistressave, sistressmax, fracLut) and decadal (masso, thkcello,
tauvo, so, volo, thetao, tauuo) need time = bnds[:, 0], not the
bnds midpoint. Clears 10x TIME001.
- DRS region token: stop lower-casing parts[4] of the compound when
building the filename and the directory path. CMIP7's region CV
carries lowercase for simple codes (glb, nh, sh) but uppercase for
latitude-band tokens (30S-90S, 30N-90N, ...). Forcing lowercase
trips wcrp FILE001 (DRS directory + filename) and ATTR004 (region
CV) for the 1hr/3hr south-30 hemispheric files. Preserves whatever
case the compound carries; the data-request corpus is the source
of truth. Clears 36x FILE001 + 18x ATTR004 region.
Source files from OpenIFS-XIOS / NEMO-XIOS / FESOM ship ``time_counter`` as the time dim. Pycmor only renamed it to ``time`` when each rule set ``time_dimname`` explicitly; otherwise time_counter survived load and the downstream behaviour was inconsistent across paths: - The primary loader (core.gather_inputs.load_mfdataset) skipped its year filter because ``"time" in dims`` was False. - The secondary-input loader (custom_steps._load_secondary_mf*) ran a drop_vars block that REMOVED time_counter from coords without first renaming it. The secondary returned without any time coord. - Custom compute_X steps then did primary × secondary arithmetic where the primary still carried time_counter and the secondary carried no time at all. xarray broadcasting collapsed the result to a single time stamp at best, an empty time dim at worst. cli72 surfaced this on ``sfcWind_1hr_south30``: output file shipped ``time: 0`` (zero-length unlimited dim) with no time variable, tripping 8 HIGH (ATTR004 coordinates, DIM002, cf §5, cf §5.1 each twice). Add auto-detect for time_counter / time_centered in both loaders so the rename happens without each rule having to opt in. Explicit ``time_dimname`` still wins. Companion bnds variables (time_counter_bounds, time_counter_bnds) get renamed alongside the dim.
Two rules sharing cmor_variable + branding_suffix + frequency but differing in region (rlds_1hr_south30 vs rlds_1hr glb, rsds_1hr_south30 vs rsds_1hr glb, ...) used the same _rule_files prefix, so each rule's qc step picked up the other rule's files. On 1hr / 3hr / 6hr regional rules this raced against the glb rule's concurrent cmip7repack: the south30 rule's strip cleared _QuantizeBitGroom* on the glb file, then the glb rule rewrote that file (re-introducing the attr transiently), then the south30 cchecker read the stale state and the sidecar recorded a §2.3 _QuantizeBitGroom* finding that wasn't actually on disk. Anchor _rule_files on <var>_<branding>_<freq>_<region>_ so each rule only sees its own files. The DRS filename layout <var>_<branding>_<freq>_<region>_<grid>_... makes the region token a clean prefix boundary. Clears the 1× §2.3 finding on cli72 (rlds_1hr_south30) and removes the class of stale-finding races on every south30 / hemispheric rule.
cmip7repack writes ``<file>.nc_cmip7repack``, then atomically renames it over the original. If the wrapping SLURM job times out mid-repack (3 h walltime, daily 137-level 3D atm fields like cl_day / pfull_day each take well over an hour), cmip7repack gets killed between "successfully created" and the rename. The partial intermediate stays on disk forever — corrupted (HDF error on open) and taking GB per file. cli72 left 30 GB of zombies (cl_day + pfull_day at 14.8 GB and 15.0 GB each). The next batch would inherit them and the disk accounting would keep climbing every cli iteration. Add ``_clean_cmip7repack_orphans`` and invoke it before AND after every ``_run_cmip7repack`` pass so: - before: a previous job's killed-rename zombies are removed before this job's repack starts (otherwise a half-baked intermediate could shadow the in-progress repack on Lustre's metadata cache). - after: any zombies left by THIS job (timeouts, OOM, signal) are removed at the end of the qc_repack step. No more 30 GB silent accumulation per batch.
Investigation of the 1hr / 3hr / 6hr atm files in cli72 found three
distinct issues, all addressed here.
1. timeaverage._get_time_method only recognised CMIP6's "Pt" suffix on
the frequency string. CMIP7 dropped the suffix and moved the
tavg-vs-tpt signal into cell_methods ("time: point" vs "time:
mean"). Every CMIP7 tpt sub-daily / monthly rule was treated as
MEAN: timeavg ran .mean() + the midpoint shift on already-instant
data, so stamps moved +3h (6hr) / +1.5h (3hr) / +0.5h (1hr) and the
on-disk cell_methods still said "time: point". The psl_6hr output
in cli72 had stamps at 15:00, 21:00, 03:00, 09:00 instead of 06,
12, 18, 00 — exactly the +3h shift, plus boundary trim.
Add an inline cell_methods check that overrides MEAN -> INSTANTANEOUS
when the data request says "time: point". The CMIP6 Pt path stays
intact; this only kicks in for CMIP7 rules.
2. ps_1hr (cap7_atm) was branded ``atmos.ps.tpt-u-hxy-u.1hr.glb`` but
used source ``atmos_1h_sfc_sp`` which the OIFS XIOS file_def
declares as operation="average" (time_centered stamps at HH:30).
CMIP7 has both ``atmos.ps.tavg-u-hxy-u.1hr.glb`` and the tpt form;
producing tpt from averaged data is wrong. Switch to the tavg
compound. The 3hr / 6hr ps rules already use the matching ``_pt_``
instant sources (atmos_3h_pt_sp, atmos_6h_pt_sp), so no change there.
3. ps_1hr_south30 (extra_atm) and ts_3hr (extra_atm) hit the same
source mismatch but with no tavg compound available in the CMIP7
registry for that region/frequency. Disabled with TODO comments
pointing at the OIFS XIOS config that would need to add the
``atmos_1h_pt_sp`` / ``atmos_3h_pt_ts`` instant fields. Re-enable
once the source data exists.
Both are CMIP7 HIGH-priority variables but the OIFS XIOS file_def in this production run only emits averaged sources where CMIP7 asks for ``time: point``: - atmos.ts.tpt-u-hxy-u.3hr.glb wants instants to capture diurnal- cycle peaks; we only have ``atmos_1h_ts_ts`` (averaged at HH:30). - atmos.ps.tpt-u-hxy-u.1hr.30S-90S wants instants to capture sub- hourly Southern-Ocean pressure-wave variability; we only have ``atmos_1h_sfc_sp`` (averaged at HH:30). Sampling the averages and labeling them as instants would ship the wrong data type — the variability the variables are meant to capture isn't in the source. Production is already underway, so adding the XIOS fields would require a model restart from an earlier checkpoint. Skip them honestly. variable_coverage.md is updated with the gap explanation; YAML rules stay commented out with an inline note.
Two MEDIUM cleanups for cli73: - variable_attributes ``is_temperature_sn`` now accepts compound CF standard names that don't strictly end with ``temperature`` (e.g. ``sea_water_potential_temperature_at_sea_floor`` on tob, ``sea_water_conservative_temperature``, ``sea_surface_temperature``) via an ``_temperature`` substring test. Also accepts ``degC`` / ``Celsius`` as absolute-temperature units. Clears CF §3.1.2 ``tob`` units_metadata MEDIUM. - When the data-request ``cell_measures`` is the CMIP7 ``--MODEL`` placeholder, substitute the realm default rather than dropping it: ``area: areacello`` for ocean / seaIce / landIce / ocnbgchem, ``area: areacella`` for atmos / aerosol / land / atmosChem. wcrp ATTR001 requires the attribute to be PRESENT, and the existing placeholder-drop left it absent on siu / siv / sidmasstranx / sistrxdtop and similar variables whose data-request entry says ``--MODEL``. Clears 8x ATTR001 cell_measures MEDIUM. §2.4 dim-order MEDIUMs on unstructured-grid files (mrsll, zg, hur, ua_6hr, tsl, mrsol, ...) NOT touched. The CF check classifies single- horizontal-dim variables as T,Z,(A) and reports they're not in T,Z,Y,X order. Same shape affects ROMS curvilinear in compliance- checker's own test corpus; upstream limitation, not real non- conformance. Leave as advisory MEDIUM.
FESOM unstructured triangles crossing the dateline ship vertices on both branches (e.g. (179°, -179°, -180°)) while FESOM's element centroid stores ``lon`` in one branch only. cf §7.1 then sees the centroid outside the [min, max] vertex bbox, reporting "N points lie outside the bounding box". cli73 cleared most of the FESOM yearlies but still tripped on 1814 elements per file for difmxylo / tauuo / tauvo / sistressave (~0.03 % of cells, all on the dateline). This isn't a precision issue — both ``lon`` and ``lon_bnds`` are already double on disk (cli67 fdf444f promotion was symmetric across lat and lon). lat passes because it doesn't wrap. The real fix is geometric: shift each vertex into the 360° window centred on the centroid, identity on the sphere, no change to the represented geometry. After the shift the bbox contains the centroid by construction. Implemented at the tail of ``_ensure_lat_lon_bounds_impl`` so it applies to every path that lands lon_bnds in the dataset (XIOS rename, _recover_bounds_from_inputs, _attach_bounds_from_mesh, add_bounds_from_coords). Vectorised — the 6.2M-element 3D files don't churn Python. Verified two ways: - Direct math on cli73 difmxylo on-disk file: 1814 → 0 outliers. - Isolated test through _ensure_lat_lon_bounds_impl on a 5-cell synthetic dataset with two dateline crossings: 2 → 0 outliers, non-dateline cells (and all of lat_bnds) untouched, log line emits the expected "shifted N rows" count.
…ine drop
- files.py: sub-daily yearly-split files emit 6-digit YYYYMM-YYYYMM token;
sidesteps cc-plugin-wcrp \d{6}|\d{8} regex (PR #46). Yearly path
already used 8-digit for the same reason; sub-daily clamps DOWN to 8
was the natural symmetric choice but I picked 6 here to match the
minimum-distortion encoding given we always split yearly.
- time_bounds.py: detect bnds straddling time stamps
(bnds = (t-dt/2, t+dt/2), the OIFS-XIOS sub-daily tavg convention)
and shift back by dt/2 so bnds = (t-dt, t) lands on the canonical
CMIP period-aligned layout. Triggers only for freq in {1hr,3hr,6hr}
with cell_methods != time:point. Unblocks TIME001 once wcrp#51 lands.
- timeaverage.py: xarray's DataArrayResample.first() does not accept
the flox engine kwarg. Drop it from the INSTANTANEOUS branch.
Latent bug exposed by 6c0a3c5's CMIP7 tpt cell_methods override:
before that commit every CMIP7 tpt rule fell through to MEAN because
CMIP6's Pt suffix was dropped, masking the TypeError.
…pattern
Group A (4 shards, ValueError on reset_coords for indexed time dim):
- time_bounds.py: short-circuit on freq in (fx, ofx); time-invariant
vars should not get bnds built. Without this, fx fell through the
wcrp_treats_as_instantaneous branch and built (12,2) monthly bnds
against the source's repeated monthly LSM samples.
- files.py: fx save branch now squeezes the time dim unconditionally
(was guarded on time.size <= 1, which missed the 12-sample case)
and uses drop_vars on the time/time_bnds coords instead of
reset_coords. reset_coords refuses indexed dim coords; drop_vars
removes both the variable and its index in one step.
Group C (thetao_ga compound mismatch, 1 shard):
- Bump CMIP7_DReq_metadata from v1.2.2.3 to v1.2.2.4 across all 17
recipe yamls. Between those versions the ocean global-average
compounds were renamed tavg-ol-hm-sea -> tavg-u-hm-sea (thetao,
bigthetao, so) and atmos surface-air-temperature 1hr files moved
from realm "land" to "atmos".
- extra_land + cap7_land: rename
land.tas.tavg-h2m-hxy-u.1hr.{glb,30S-90S} ->
atmos.tas.tavg-h2m-hxy-u.1hr.{glb,30S-90S}.
Group E (extra_atm script:// hashing, 1 shard, watchdog killed):
- pipeline.py: detect steps whose __module__ is "script" (the
synthetic name from core.utils.get_function_from_script) and use
NO_CACHE for that pipeline. Prefect's INPUTS cache policy pickles
every task arg (Rule + bound steps), and pickle fails on the
@lru_cache wrappers defined in custom_steps.py because "script"
isn't a re-importable module name. Falling back to NO_CACHE costs
nothing — pycmor runs each pipeline once per process.
Group D (sisnhc_day MemoryError on 1.06 PiB allocation):
- veg_seaice yaml: tighten regex pattern from m_snow.fesom..*\.nc to
m_snow\.fesom\.\d{4}\.nc, same for a_ice. The loose form matched
both gn and gr-variant files (m_snow.fesom.1851.nc AND
m_snow.fesom.gr.1851.nc), and xr.open_mfdataset tried to merge
them via combine_by_coords, which triggered dask.tokenize on a
degenerate-shape metadata array — hence the 1.06 PiB "allocation"
was actually a tokenize trying to hash an impossible reshape.
cli78 round 2 of Group A. 3bd1684 stopped reset_coords from failing on the indexed time dim but left ``extra_kwargs["unlimited_dims"] = ['time']`` in place (set earlier in _save_dataset_impl when the rule has ``xarray_time_unlimited: True``). xr.save_mfdataset then rejects the kwarg against a dataset that no longer has a 'time' dim. Drop the kwarg in the fx branch right after the coord drop. Verified mrfso open question from the handoff: only the genuinely-fx rules hit the branch (mrsofc, sftgif, rootd, sftlf, plus core_land and extra_land LPJ fx). mrfso runs through lpjg_monthly_pipeline and saves cleanly — the earlier handoff misidentified it.
cli79 still surfaced 2 HIGH '[ATTR004] sob units' findings. Root cause:
the DReq path skips setting units (skip_setting_unit_attr=True in this
profile), so source-inherited units pass through to the file. FESOM salt
ships units='1e-3' from XIOS, and the wcrp registry literal-compares
against '1E-03'. The earlier psu->'1E-03' substitution at line 101-109
only runs against the DReq attrs dict, never against the source-inherited
da.attrs.
Add a second normalisation pass after da.attrs.update(attrs) that
canonicalises whatever ended up on the array: 'psu' -> '1E-03' (defence
in depth, in case DReq didn't ship it) and '1[eE]-0?\d' -> uppercase
two-digit-exponent form. Tested against {1e-3, 1E-3, 1e-03, 1E-03, psu,
m s-1 psu, kg m-2, kg m-2 s-1}; the four equivalent forms all collapse
to '1E-03', the others are untouched.
Clears the 2 sob HIGH from QC_REPORT_cli79.md.
Closes most of QC_REPORT_cli79.md's actionable items.
msftm cluster (~13 HIGH):
- coordinate_metadata.yaml: add 'lev' entry (same shape as olevel) so the
set_coordinates step recognises the depth dim that compute_msftmz emits
as 'lev'.
- bounds.py: add 'rho' to the vertical_coord_names default list so
add_vertical_bounds builds rho_bnds for msftm density variables.
- lrcs_ocean recipe: add add_vertical_bounds + set_coordinates to the 4
msftm pipelines (msftmz, msftm_density, msftmmpa_depth,
msftmmpa_density). They were emitting rho/basin/lev coord vars with no
CF attrs at all because set_coordinates was never called.
§7.2 cell_measures (~67 MEDIUM):
- variable_attributes.py: when DReq supplies cell_measures='' (canonical
for hm/scalar quantities), drop the attribute from both attrs and
da.attrs. cf-checker §7.2 regex
``^(?:area|volume):\s+\w+$`` rejects empty strings; WCRP registry has
no value for cell_measures on these variables either. Absence passes
both checks.
- variable_attributes.py: when cell_measures is the combined
``area: areacello volume: volcello`` form (DReq emits this for ocean
3D), trim to just ``area: areacello``. The same cf-checker regex
doesn't match the combined form even though CF §7.2 itself allows it.
§7.1 soil-layer bounds (3 MEDIUM):
- custom_steps.py: load_lpjguess_monthly_depth constructed sdepth_bnds
with attrs={'long_name': 'depth layer boundaries', 'units': 'm'}.
CF §7.1 requires bounds variables to inherit boundary-related attrs
from the parent coord, not carry their own. Set attrs={} on
sdepth_bnds, same shape as the time_bnds construction in time_bounds.py.
Affects mrsll_mon, tsl_mon, mrsol_mon (all three use the same loader).
Closes the portability gap external collaborators reported. blosc-encoded files cannot be opened without hdf5plugin / a matching libhdf5 plugin build, and we have already paid that cost in-house: HANDOFF_memory_pressure.md documents that neither NCO 5.0.6 (spack) nor NCO 5.3.3 (conda) on Levante can read our cli output even with HDF5_PLUGIN_PATH set; we wrote pyconcat specifically as a workaround. Drop the 'netcdf_compression_codec: blosc_zstd' + 'netcdf_compression_level: 3' overrides from the inherit blocks of all 17 production tier yamls plus 3 example yamls. The pycmor fallback at files.py:1147 is already 'zlib' and the config default at config.py:481 is level 1, matching CMIP6 archive convention. BitGroom-5 quantization is independent of codec and stays on (default in files.py:1152, no recipe override), so lossy compression is unchanged. The byte-level wrapper around the already-quantized data is what changes. Perf: bench_hr_ua_6hr_results.md / OPTIMIZATION_PLAN.md concluded 'compression is innocent' — disabling compression made wall slower, not faster, because page cache scales with file size. Switching codec is perf-neutral at this scale. Expected size inflation per HANDOFF_netcdf_compression_codec.md: ~10-30% vs blosc_zstd-3 on HR climate data. Bracket on real data with 'nccopy -d 1 -s' before relaunching. The 18 remaining cmip7_bench_hr_ua_6hr_*.yaml fixtures keep their per-rule blosc_zstd overrides untouched: they are historical A/B benches for the memory-pressure investigation, changing them would invalidate the documented baselines.
Closes the two genuinely fixable HIGH findings from cli81:
VAR004 Bounds for 'rho' (msftm cluster, 1 HIGH):
Pipeline order in the five vertical-bnds pipelines (4 msftm + the
existing ocean_vertical_integration) had add_vertical_bounds BEFORE
timeavg. xarray's Dataset.resample(time=...).mean() broadcasts every
data variable that lacks the time dim onto a fresh time axis, so
rho_bnds(rho, bnds) emerged from timeavg as
rho_bnds(time, rho, bnds) — the broadcast is what the cli81 finding
('Found (12, 89, 2)') flagged. Same shape bug applies to lev_bnds in
the ocean-level variant.
Fix: reorder to timeavg → set_time_bounds → add_vertical_bounds.
Add the vertical bnds AFTER the time axis has been resampled, so they
inherit the final shape (89, 2) instead of getting picked up by the
resample.
Also: bounds.py:calculate_bounds_1d was writing
attrs={'long_name': '<coord> bounds'} on the bnds DataArray. CF §7.1
forbids bnds vars from carrying boundary-related attrs; drop to
attrs={}.
§3.3 Standard Name on time_bnds (1 HIGH):
cf-checker §3.3 fires on aux-coord variables that carry neither
long_name nor standard_name. The previous CF §7.1 fix stripped both
from time_bnds. The two checks have opposite-looking demands but
reconcile if bnds attrs MATCH the parent: §7.1 wants no mismatch,
§3.3 wants a name present.
Fix in time_bounds.py: instead of attrs={}, copy long_name from the
parent time coord onto time_bnds. Both freshly-built (line 281) and
existing-bnds (line 205) paths now do this. §3.3 passes (long_name
present), §7.1 passes (matches parent).
Counterintuitive findings from the cli80 vs cli81 diff:
- §7.1 soil-layer (3 MEDIUM in cli79) already cleared between cli79
and cli80; the 126f4d7 sdepth_bnds attrs={} change was already
effective in cli80 (no regression in cli81 vs cli80).
- §7.2 cell_measures fix from 126f4d7 swapped the 24 cf §7.2
findings into 24 wcrp ATTR001 'attribute missing' findings on
siarea/siextent/sisnmass/sivol. Net MEDIUM count unchanged. The
three checks (cf §7.2 format, wcrp ATTR001 existence, wcrp ATTR004
registry value) have mutually-exclusive demands for variables where
the registry has no cell_measures value; clearing requires upstream
coordination, not a pycmor-side fix.
…llow-up) cli90 verified that the cc-plugin-wcrp #52 upgrade is live (TIME001 now reports midpoint=True), but the 57 sub-daily TIME001 HIGH did not clear. Mismatch is at the same value (0.020833 expected vs 0.041666 got), the file structure unchanged from cli82. Root cause: _bnds_centered_on_time and _shift_bnds_half_step_backward only handled np.datetime64 and cftime-object bounds. FESOM/XIOS time_bnds ship without a units attribute, so xarray leaves them as float 'days since ...'. The float path fell into the cftime branch and hit Exception on float.total_seconds(), silently returning False. The log line 'shifted time_bnds back by dt/2' never printed; the bnds and midpoint stayed straddled around the timestamp. Add a float branch: - _bnds_centered_on_time: detect float dtype, compare halves in days, 1 minute tolerance = 1/1440 day. - _shift_bnds_half_step_backward: same vectorised path as datetime64 for any floating dtype. Smoke test on cli90 1hr tas data: centered? True shifted bnds: [[-5e-09 0.04166], [0.04166, 0.08333], ...] midpoint of shifted: [0.02083, 0.0625, 0.10416] = 00:30, 01:30, 02:30 That is the canonical CMIP layout TIME001 (midpoint=True) expects. cli91 should drop the 57 sub-daily TIME001 HIGH cleanly. Remaining HIGH: 1 basin (#49) + 2 msftm lev (our code follow-up).
cli82 STATE doc identified the msftm 'ol' (ocean-level) file as the last fixable HIGH cluster (2 findings on msftm_tavg-ol-hyb-sea_mon_glb_g130_*.nc): [VAR005] Coord monotonicity for 'lev' [VAR012] Bounds value consistency for 'lev' Root cause: compute_msftmz and compute_msftmmpa_depth pull the vertical axis from FESOM's mesh.zlev, which stores depths as negative-down (0, -5, -10, ..., -6250 m). The file then declares lev:positive='down' and lev:standard_name='depth', which together demand POSITIVE values increasing with depth. wcrp's VAR005 (monotonicity) and VAR012 (bounds-value consistency) both flag the sign mismatch. Fix: negate at both sites (compute_msftmz line 4467, compute_msftmmpa_depth line 5146). On-disk coord becomes 0, 5, 10, ..., 6250 — monotonic increasing under positive=down, matches lev_bnds direction, satisfies both checks. Expected cli91 vs cli90 HIGH delta (combined with 22ba44a OIFS-XIOS float-dtype fix): TIME001 sub-daily: 59 -> 0 (22ba44a) basin geophysical: 1 -> 1 (upstream cc-plugin-wcrp#49) VAR005 lev msftm: 1 -> 0 (this commit) VAR012 lev msftm: 1 -> 0 (this commit) TOTAL HIGH: 62 -> 1 (only basin remains, upstream-blocked)
Root cause of every cli66-cli91 sub-daily TIME001 failure traced to src/pycmor/std_lib/time_bounds.py lines 734-737 (Paul Gierz 02d89e6, 2026-04-01). The 'default: consecutive time points as bounds' branch built bnds[i] = (t[i], t[i+1]) — straddling the source timestamp rather than centered on it. For input at HH:30 (the canonical CMIP midpoint), that yielded bnds = (HH:30, HH+1:30); the downstream ``new_time = midpoint(bnds)`` then shifted time forward to HH+1:00. Source OIFS-XIOS files are already canonical: time_counter = 00:30, 01:30, 02:30, ... (midpoints) time_counter_bnds = (00:00, 01:00), (01:00, 02:00), ... Pycmor was breaking the canonical layout. Output had: time = 01:00, 02:00, 03:00, ... time_bnds = (00:30, 01:30), (01:30, 02:30), ... Fix: build bnds centered on the input timestamp: bnds[i] = (t[i] - dt/2, t[i] + dt/2) For input at HH:30 with dt=1h: bnds[0] = (00:00, 01:00) — canonical, midpoint = HH:30 = input. The earlier 3e09122/22ba44a2 attempt to fix this lived in the existing-bnds branch and only fired when source bnds had no units attr; it was also based on a misdiagnosis (assumed XIOS shipped centered bnds), so removed entirely along with the now-unused helpers _bnds_centered_on_time / _shift_bnds_half_step_backward. Smoke test: input = [00:30, 01:30, 02:30, 03:30] bnds[0] = (00:00, 01:00) midpoint = [00:30, 01:30, 02:30, 03:30] == input ✓ Expected cli92 vs cli91: TIME001 sub-daily: 59 -> 0 basin geophysical: 1 -> 1 (still cc-plugin-wcrp#49) TOTAL HIGH: 60 -> 1
Hardens against the Levante AssocMaxJobsLimit-induced cancellations
that cost us one shard in cli91 (lrcs_land_1 went CANCELLED+ after
pending too long under the 20-job-per-association cap).
submit_hr_year_shards.sh:
- Add ARRAY_MAX_CONCURRENT env knob (default 4). Each per-tier sbatch
array gets ``--array=1-N%${ARRAY_MAX_CONCURRENT}`` so no single
array can monopolise all 20 association slots and starve the
smaller arrays into the cancel-after-pending zone. Single-shard
arrays skip the %cap (keeps log noise down).
resubmit_cancelled_shards.sh:
- New companion script. Reads the submit log next to the workdir,
walks each tier's sbatch jobid via sacct, identifies any task in
CANCELLED* state, and resubmits each as its own size-1 array
pointing at the same shard yaml. Run after a sweep finishes to
catch any losers.
Usage:
resubmit_cancelled_shards.sh /scratch/$USER/pycmor_hr/cli92_full 1851
cli93 regressed badly (119 HIGH vs 60 in cli91) because the centered-bnds fix from 3976a28 interacted with the wcrp_treats_as_instantaneous branch in set_time_bounds. The pycmor-side AVG list was the pre-#52 version (no 1hr/3hr/6hr), so sub-daily tavg files were classified as instantaneous and got time = bnds[:, 0] (period_start) instead of time = midpoint(bnds). Combined with the centered bnds = (t-dt/2, t+dt/2) build, the period_start assignment shifts time backward by dt/2. For source at 1851-01-01 00:30 that puts the new time at 1851-01-01 00:00, then the FIRST stamp's bnds[0] = (1850-12-31 23:30, 1851-01-01 00:30) straddles the year boundary — file-split logic then peels the first stamp into a 1850-named orphan file (1 stamp in clt_..._18500101-18501231.nc, 8759 stamps in the 1851 file). cc-plugin-wcrp#52 (merged 2026-06-25 to master, beb00ce) extended AVERAGE_CORRECTION_FREQ to include 1hr/3hr/6hr. With the wcrp plugin upgrade live since the cli90 turn, the pycmor mirror has been stale. Sync it: add 1hr/3hr/6hr to _WCRP_AVG_FREQS. Sub-daily tavg files now take the "mean" path, time = midpoint(centered bnds) = input time (zero shift), file-split keeps everything in 1851. Expected cli94 vs cli93: TIME001 sub-daily: 87 -> 0 TIME003 (filename coverage): 31 -> 0 basin geophysical: 1 -> 1 TOTAL HIGH: 119 -> 1
Fixes the pycmor-side issues Martin Schupfner flagged in his review of
the cli94 files. Each item is either a direct spec-compliance fix or a
defensive cleanup matched to published CMIP7 guidance.
Global attributes (global_attributes.py):
Drop CMIP7-eliminated attrs from required_global_attributes per
Appendix 2 of the CMIP7 Global Attributes guidance:
- further_info_url (link doesn't resolve; obsolete es-doc reference)
- grid (free-text; source_id + grid_label cover the intent)
- source_type (AOGCM-style label; no CMIP7 role)
- sub_experiment (always "none" for our runs; noise)
- sub_experiment_id (kept for internal DRS path use, not written)
- table_id (CMIP6-style "A1hr"; CMIP7 uses "atmos"/etc., not checked)
- source (required in CMIP6, optional in CMIP7; our value was
"<source_id> <realm>" which duplicated source_id)
Also add _sanitize_attr_value to translate em-dash (U+2014), en-dash
(U+2013), figure-dash (U+2012) to ASCII hyphen. Older libhdf5 builds
can mis-encode these in attribute strings across writer/reader locale
boundaries.
Filename time tokens (files.py):
Emit CMIP7 Appendix 1-mandated precisions:
yr/yrPt/dec YYYY-YYYY
mon/monC/monPt YYYYMM-YYYYMM
day YYYYMMDD-YYYYMMDD
6hr/3hr/1hr/*hrPt YYYYMMDDhhmm-YYYYMMDDhhmm
subhr/subhrPt YYYYMMDDhhmmss-YYYYMMDDhhmmss
Previous revs clamped everything to 6/8-digit to fit the pre-#46
wcrp TIME003 regex. That was spec-violating. cc-plugin-wcrp#46 must
land before sub-daily/subhourly files pass the checker.
Time bounds (time_bounds.py):
A. Skip time_bnds creation for time_method="instantaneous" (tpt).
CMIP7 output requirements (matching CMIP6 practice) forbid bounds
on instantaneous variables. Also drop any source-supplied bnds and
clear the parent time coord's ``bounds`` attribute.
B. Stop emitting the int64 bnds(bnds) index helper on new
bnds vars. It was added to give cf §3.3 a metadata target but is
unusual for CMIP output and adds no value.
Coordinates attr (coordinate_attributes.py):
Only list AUXILIARY (non-dim) coordinates in ``coordinates`` on data
variables. Dim coords are implicit per CF. When no aux coord exists,
drop the attribute entirely rather than emit a stale value inherited
from source.
cli95 vs cli94: HIGH 46 → 100, MEDIUM 224 → 763. Two of the three Schupfner-review items in f7cbcda backfired. Rolling them back with better implementations. Item 2 (filename token: 57 TIME003 findings cleared) f7cbcda built the filename token from ``time[0]`` / ``time[-1]``, which for tavg sub-daily land on midpoint stamps (HH:30). The CMIP7 spec says the token describes COVERAGE, not the timestamps. Fix: prefer ``time_bnds[0, 0]`` / ``time_bnds[-1, 1]`` (period edges) when bounds exist, fall back to ``time[0]`` / ``time[-1]`` for tpt files that carry no bnds. Token now reads e.g. ``185101010000-185201010000`` instead of ``185101010030-185112312330``. Still needs cc-plugin-wcrp#46 to land for the 12-digit form to clear pre-#46 regex, but the semantic is now spec-correct regardless of checker version. Item 3 (tpt time_bnds: 42 TIME001 findings cleared for LUT yearlies) f7cbcda skipped set_time_bounds entirely whenever ``time_method=="instantaneous"``. That also skipped the time realignment (``time = bnds[:, 0] = period_start``) that the wcrp_treats_ as_instantaneous branch does, so LPJ-GUESS LUT yearly tpt files (cLitterLut, cSoilLut) ended up with time stamps at mid-year instead of Jan 1. TIME001 then flagged 42 files with ``expected -181.000000, got 0.000000`` because the checker's theoretical time (from filename_start) disagreed with the mid-year stamp. Fix: keep the full flow (so time gets realigned), then at the END drop time_bnds only if the CMIP7 axis for this variable's temporal_shape has ``must_have_bounds="no"``. Only ``time-point`` (→ time1 axis) maps to "no"; everything else keeps bnds. Vendored ``CMIP7_coordinate.json`` from WCRP-CMIP/cmip7-cmor-tables main (cmor_version 3.15, table_date 2026-07-01) at ``src/pycmor/data/cmip7/CMIP7_coordinate.json`` so lookups don't hit the network. Bump the vendored copy when the upstream table date changes. Projected cli96 vs cli95 with both fixes: TIME003 filename: 57 → 0 (post-#46; unchanged pre-#46) TIME001 LUT: 42 → 0 TOTAL HIGH: 100 → 1 (basin, still #49)
cli96 (0b7714c) tripped 522 TIME003 findings because _filename_time_range used bnds[-1, 1] (right edge of last period, = start of next period), but the checker's _coverage_from_time uses bnds[-1, 0] (left edge of last period, = the last period's own start). Every filename slid one period past coverage: yr 1851-1852 (want 1851-1851) mon 185101-185201 (want 185101-185112) day 18510101-18520101 (want 18510101-18511231) 1hr 185101010000-185201010000 (want 185101010000-185112312300) Independent of cc-plugin-wcrp#46: mon and day tokens still 6/8-digit and parse on the currently installed wcrp regex; only the shape of the end edge was wrong.
wcrp ATTR001 requires _FillValue and missing_value on every variable, including flag-valued integer variables like basin/siline. The previous code skipped flag variables entirely because 1e20 does not round-trip through int32. Use NC_FILL_INT (-2147483647) for both attr and encoding on flag variables instead. Basin flag values (1, 2, 3, 10, 11) sit well inside the int32 range so the sentinel does not collide with data. Kills 2 of the 3 basin fx HIGH findings (cli100). The VAR005 int-vs-float type expectation is upstream; CMIP7 says flag variables ARE integer.
CMIP7DataRequestVariable stored _temporal_shape and _spatial_shape as private fields but never exposed them via property getters. Downstream consumers using getattr(drv, 'temporal_shape', None) silently got None, which sent time_bounds._axis_must_have_bounds down the default-True branch. Result: tpt (time-point) files never had their time_bnds dropped, the filename token derived from bnds[0, 0] (period start, e.g. 01:30 for 3hr), the checker's TIME001 saw the actual stamp at the canonical position (e.g. 03:00), and every 3hr tpt file tripped HIGH with expected=0.0625 / got=0.125. Smoke test: hurs 3hr tpt (CF3hr.hurs, time-point) → _axis_must_have_bounds=False ✓ tas mon tavg (Amon.tas, time-intv) → _axis_must_have_bounds=True ✓ Kills 25 TIME001 HIGH findings from cli100.
CMIP7 Appendix 1 (Global_Attributes) says the filename time token is built from the ``time`` coordinate values, not from ``time_bnds``. This matches CMOR practice. DKRZ review (Martin Schupfner, 2026-07-06) and cc-plugin-wcrp #62 (sol1105) both point the same way: coverage source is ``time[0]``/``time[-1]``, with ``strftime`` truncating to the frequency's precision. Reverts 69b37e8's ``bnds[-1, 0]`` end. That was aligned to the pre-#62 checker but shipped ``185101010000-185112312300`` for 1hr tavg where Appendix 1 wants the midpoint ``185101010030-185112312330``. Verified against cli101 samples: 1hr tavg tas: old 185101010000-185112312300 → new 185101010030-185112312330 mon tavg tas: 185101-185112 unchanged (mid-month day truncates to month) day tavg tas: 18510101-18511231 unchanged yr tavg cropFrac: 1851-1851 unchanged 3hr tpt hurs: 185101010300-185112312100 unchanged (tpt already used time)
CMIP-IPO released v1.2.2.5 on 2026-07-17. Bump vendored DReq exports
and default version strings across the pycmor sources.
Data files:
* all_var_info.json regenerated with export_dreq_lists_json v1.4
for v1.2.2.5. Format shift: keys are now CMIP7 compound names
(atmos.hurs.tpt-h2m-hxy-u.3hr.glb) instead of CMIP6-era short
names (CF3hr.hurs). Entries gained cmip7_compound_name /
cmip6_compound_name / branding_label / branded_variable_name /
region / uid / processing_note fields; dimensions field now names
the canonical axis (time1) instead of the temporal_shape.
Variable count 1813 -> 1968. Regional variants (30S-90S, ata,
grl, nh, sh) newly represented.
* CMIP7_coordinate.json refreshed from WCRP-CMIP/cmip7-cmor-tables
main (table_date 2026-07-01 -> 2026-07-09). Same cmor_version 3.15.
time1 must_have_bounds=no confirmed unchanged.
Code default version strings v1.2.2.2 -> v1.2.2.5 in:
* cli.py (--cmip7-dreq-version arg default)
* core/resource_locator.py (DEFAULT_VERSION + docstrings)
* core/validate.py (schema default comment)
* core/cmorizer.py (YAML example)
* data_request/cmip7_interface.py (load_metadata / get_cmip7_interface
defaults, docstring examples)
Smoke test: 9/10 production compound names from cli101 yamls resolve
directly (was 0/10 pre-upgrade because keys were CMIP6-era). The one
miss (atmos.tas.tavg-h2m-hxy-u.1hr.30S-90S) is our rule using a region
variant the DReq does not enumerate; that behaviour is unchanged and
handled elsewhere in the pipeline.
Short walkthrough for a colleague running their first pass on their own model output. Covers env setup against the shared miniforge, rule copy from awi-esm3-veg-hr-variables, data_path repointing, local smoke test, full year on SLURM via submit_hr_year_shards.sh, QC aggregation, and common HIGH-finding fixes. Includes gotchas around SLURM account, multi-node memory, and known upstream-side checker findings.
_load_secondary_mf (and the legacy uncached variant) read year_start/
year_end off the rule to narrow broad secondary ``*_pattern`` regexes
(aice_pattern, salt_pattern, sgm22_pattern, ...) at load time. Those
attrs are only set by the CLI --year-start/--year-end overrides, which
run_hr_shard.sh passes but a bare ``pycmor process`` (run_hr_yaml.sh
path) does not. repoint_hr_year.py year-locks only three enumerated
keys (pattern, second_input_pattern, hnode_pattern) in the yaml text,
so under the bare runner the other twelve ``*_pattern`` families opened
every year on disk - cli51-style year mixing across ~50 secondary
loads.
repoint already injects ``year: <YYYY>`` into the inherit block. Use it
as the fallback when year_start/year_end are absent:
year_start = rule.get("year_start", rule.get("year"))
year_end = rule.get("year_end", rule.get("year"))
Explicit CLI overrides still win; rules with none of the three attrs
keep the old unfiltered behaviour.
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
3 participants
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.
CMIP7 cmorization for AWI-ESM3-VEG-HR
Adds full CMIP7 support targeting AWI-ESM3-VEG-HR, including a native compound-name
architecture that replaces the legacy cmip6-table-based data request lookup.
Key changes
CMIP7 data request
DataRequestfromCMIP7_DReq_metadataJSON instead of cmip6 tablesocean.tos.tavg-u-hxy-sea.mon.GLB)cmip6_table→cmip6_cmor_tablein vendored metadatacompound_namematching againstcmip6_compound_nameandcmip7_compound_nameattributestable_idfrom compound name when not set explicitlyValueErroron zero DRV matches (instead of silent skip)Pipeline
vertical_integratecustom pipeline stepconvert()step fromDefaultPipelineStateobjects not being unwrapped to actual results in parallel runsStandard library
src/pycmor/std_lib/time_bounds.py)getattr+_pycmor_cfgfallbackglobal_attributesto derivetable_idfrom CMIP6/CMIP7 compound namesXarray accessor API
StdLibAccessorwith.process()Test infrastructure
pycmor.fixtures.model_runs)pycmor.tutorialdataset system (xarray.tutorial-style API)Misc fixes
entry_points()compatibilitypyfesom2imports for environments without itTest plan
pytest tests/unit/pycmor process examples/awiesm3-cmip7-minimal.yamlruns successfully on Levante