fix: cast lat/lon coordinates to float64 at CMOR writer level#270
fix: cast lat/lon coordinates to float64 at CMOR writer level#270siligam wants to merge 3 commits into
Conversation
Geographic coordinate variables (lat, lon, and their bounds) must be float64 per CMOR3 and CF Conventions §2.2. Since pycmor writes via xarray rather than the CMOR C library, this must be enforced explicitly. Adds _cast_geo_coords_to_float64() called in save_dataset() so the requirement is met automatically for all pipelines. Closes #269 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Conflict notes for Jan (#266 rebase)When rebasing 1. Import — trivial 2. 3. Call site in # CMOR3 / CF requirement: geographic coordinates must be float64
da = _cast_geo_coords_to_float64(da)right after 4. Watch out: bounds created by Suggested fix: call ds_temp = _ensure_lat_lon_bounds_and_external_vars(ds_temp, rule)
ds_temp = _cast_geo_coords_to_float64(ds_temp) # ensure bounds are also float64 |
…ss CI lint check Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pgierz
left a comment
There was a problem hiding this comment.
Looks good, although I have one suggestion that might be interesting to look at.
| ) | ||
|
|
||
|
|
||
| def _cast_geo_coords_to_float64(da): |
There was a problem hiding this comment.
Out of curiosity, can this be done with xarray's "encoding" trick? Basically you can set a dict of specific properties if I remember correctly, and when the data structure gets serialized to disk, it gets transformed then and not before.
Just as an idea.
Replace in-memory `.astype(np.float64)` + `assign_coords` with `variable.encoding["dtype"] = np.float64` so the cast happens at NetCDF write time rather than mutating the caller's DataArray. This uses the same xarray encoding mechanism already in place for time variables and covers all write paths (to_netcdf, save_mfdataset, split and resample code paths) without modifying any call sites. Tests renamed to `writes_geo_*_as_float64` and extended with an assertion that in-memory dtype is not mutated. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Great suggestion @pgierz — just implemented it in the latest commit. Replaced The cast is now deferred to write time only, which is cleaner and consistent with how time encoding is already handled in this same file ( Extended the tests to also assert the in-memory dtype is not mutated, which confirms the deferred behaviour. |
Summary
_cast_geo_coords_to_float64()helper instd_lib/files.pythat promoteslat,lon,latitude,longitude, and their bounds variants tofloat64before any write path insave_dataset()assign_coords) and bounds data variables (via direct assignment), handling bothxr.DataArrayandxr.DatasetinputsCloses #269
Why float64 is required
CMOR3 (the reference implementation) explicitly casts geographic coordinate variables to
float64. CF Conventions §2.2 recommends double precision for coordinate variables. Compliance checkers (PrePARE, cf-checker) and publication tools (cmip7repack) will flag or rewritefloat32coordinates.Note for Jan (#266)
feat/cmip7-awiesm3-veg-hrhas heavily modifiedfiles.py(670 additions). Once this merges intomain, please rebase that branch so it picks up this fix. The change is a small, self-contained addition with no semantic conflict.Test plan
test_save_dataset_casts_geo_coords_to_float64— parametrized overlat/lon/latitude/longitude×float32/float16input dtypes, verified output isfloat64on disktest_save_dataset_casts_geo_bounds_to_float64— parametrized overlat_bnds/lon_bnds/lat_bounds/lon_bounds, verified bounds written asfloat64test_savedataset.pytests pass🤖 Generated with Claude Code