Preserve time_bnds Variable in Ocean CMORisation Workflow

[rhaegar325]

Summary

This PR ensures that time_bnds (temporal bounds) are properly loaded, preserved, and validated throughout the ocean CMORisation pipeline. The time_bnds variable is critical for CMIP6 compliance as it defines the temporal boundaries for each time step.

Changes

Code Changes (src/access_moppy/ocean.py)

Modified select_and_process_variables() method:

1. Load time_bnds with required variables

   • Added time_bnds to required_vars during dataset loading
   • Ensures time bounds are available from the start of processing

2. Preserve time_bnds in final dataset

   • Modified dataset selection to keep both CMOR variable and time_bnds
   • Changed from self.ds = self.ds[[self.cmor_name]] to self.ds = self.ds[[self.cmor_name, time_bnds[0]]]

3. Preserve time_bnds dimensions in coordinate cleanup

   • Extended dimension gathering to include dimensions from time_bnds (e.g., nv)
   • Prevents accidental removal of coordinates needed by time_bnds

4. Validate time_bnds presence (CMIP6 requirement)

   • Added validation check to raise clear error if time_bnds is missing
   • Ensures CMIP6 compliance by making time bounds mandatory

Test Changes (tests/unit/test_ocean.py)

Added comprehensive unit tests for time_bnds handling:

1. test_time_bnds_loaded_and_preserved

   • Verifies time_bnds appears in final dataset alongside CMOR variable
   • Ensures no other data variables are accidentally retained

2. test_time_bnds_dimensions_in_used_coords

   • Confirms time_bnds dimensions (time, nv) are preserved as coordinates
   • Validates coordinate cleanup logic correctly identifies dependencies

3. test_time_bnds_missing_raises_error

   • Tests that missing time_bnds raises a clear ValueError
   • Ensures CMIP6 compliance validation works correctly

4. test_required_vars_includes_time_bnds

   • Verifies time_bnds is included in required_vars during loading
   • Confirms the variable is requested from input files

Supergrid Tests (tests/unit/test_ocean_supergrid.py)

Comprehensive test coverage for the Supergrid class ensures coordinate transformations work correctly:
Initialization & Loading Tests:

• Supergrid loads correctly with nominal resolution
• Resolution mapping works for all supported grids (100km, 25km, 10km)
• Invalid resolutions raise appropriate errors
• All cell types created with correct structure (hcell, qcell, ucell, vcell)

B-grid Extraction Tests:

• All grid types (T, U, V, C) return correct structure
• Vertices have 4 corners as expected
• Latitude/longitude coordinates properly defined

C-grid Extraction Tests:

• All grid types work with both symmetric and asymmetric memory layouts
• Symmetric mode includes all points
• Asymmetric mode correctly trims edge points (U: -1 column, V: -1 row)
• T-grid and C-grid (corner) points correctly positioned

Output Validation Tests:

• Longitude normalized to [0, 360) range
• DataArray dimensions correct: (j, i) for centers, (j, i, vertices) for bounds
• Coordinate values are sequential integers
• Vertices shape matches spatial dimensions

Error Handling Tests:

• C-grid requires symmetric parameter
• Unsupported Arakawa grids raise errors
• Unsupported grid types raise errors

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.74%. Comparing base (0eb8d0e) to head (f9f2791).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #144      +/-   ##
==========================================
+ Coverage   52.02%   60.74%   +8.72%     
==========================================
  Files          18       18              
  Lines        2722     2731       +9     
==========================================
+ Hits         1416     1659     +243     
+ Misses       1306     1072     -234     

☔ View full report in Codecov by Sentry.
📢 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]

The problem is sometimes time bounds are not available in the dataset and we need to calculate them...

[rhaegar325]

OK, I will add a function to calculate time_bnds if the origin dataset doesn't have one.

[rbeucher]

I thought we already have that. Did I miss something?

[rhaegar325]

I thought we already have that. Did I miss something?

Maybe I missed something, but I didn’t find any mechanism in the CMORiser to calculate bounds. In the atmosphere datasets we currently use (e.g. under /g/data/p73/archive/CMIP7/ACCESS-ESM1-6/spinup/JuneSpinUp-JuneSpinUp-bfaa9c5b/), time_bnds lat_bnds, and lon_bnds are already provided as variables, and the ocean datasets include at least time_bnds. we can load them with load_dataset

[rhaegar325]

The new commit should solved the issue #144,

Add automatic time_bnds calculation for CMIP6 compliance

Automatically calculate time_bnds when missing from source ocean data,
ensuring CMIP6 compliance without manual intervention.

Key Features

• Auto-detect temporal frequency (monthly/daily/yearly/irregular)
• Calculate missing time_bnds with proper month/day/year boundaries
• Preserve existing time_bnds when already present in source data
• Support wide date ranges (0000-2200) for historical and future simulations
• Handle both numpy datetime64 and cftime datetime types

Workflow Changes

• CMORisation now checks for time_bnds after loading dataset
• If missing: automatically calculates from time coordinate
• If present: preserves original values unchanged
• Final dataset includes both CMOR variable and time_bnds

Testing

Add 8 unit tests covering:

• Automatic calculation when missing
• Preservation of existing bounds
• Boundary continuity and structure validation
• Error handling for edge cases

[rbeucher]

I'm gonna need to test this one thoroughly before merging. I remember trying something like this for the time_bounds calculation and I was hitting some issues with real data. Thanks @rhaegar325

[rhaegar325]

I'm gonna need to test this one thoroughly before merging. I remember trying something like this for the time_bounds calculation and I was hitting some issues with real data. Thanks @rhaegar325

Sure, please let me know if you encounter any issues during testing.

[rhaegar325]

In the new commit, I have add warning for users if there is no bounds in the input raw data.

[rbeucher]

Can you resolve conflicts please? @rhaegar325

[rhaegar325]

@rbeucher The conflict has now been resolved.