Skip ismip7_forcing steps when final output already exists by trhille · Pull Request #1001 · MPAS-Dev/compass

trhille · 2026-09-29T18:21:38Z

This merge contains final fixes to the ismip7_forcing test group. These include:

A one-line bug fix in config parser when determining fracture forcing verison.
A check on the existence of the final processed forcing file before beginning processing each step. This will be very helpful for restarts in which compass run is executed anywhere higher than the individual step level. Without this, early steps risk being re-run if they had completed in the previous run.

Checklist

Document (in a comment titled Testing in this PR) any testing that was used to verify the changes

When an ismip7_forcing processing step runs, it previously always regenerated and overwrote its final output. The only resume logic was at the temporary remapped_* level, which is deleted once a step completes. This meant re-running a test case (after a partial timeout, or to process a different scenario) re-did all expensive extrapolate + ncremap work for steps that already finished. This commit adds an early 'final output already exists -> info-log and skip' guard to every forcing step that produces a final output, making re-runs cheap and idempotent. Changes: - Add os.path.exists(dst) check at the start of each run() method, after source resolution but before any remap work - Hoist output_file/output_path/dst computation to before the guard so the tail of run() reuses those variables - Guard placement ensures each output is checked independently (e.g. ocean_thermal per-job outputs for AIS OCX ocean choices) Affected steps (10): - Atmosphere: process_smb, process_temperature, process_runoff, process_smb_gradient, process_temperature_gradient - Ocean thermal: process_thermal_forcing (_process_ocean_forcing and _run_climatology), build_3d_thermal_forcing - Fracture: process_excess_melt, process_lake_properties, process_shelf_collapse The existing per-year remapped_* skip logic is unchanged; these guards operate at the final canonical output level. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

trhille · 2026-09-29T18:27:23Z

Testing

I ran compass run at the atmosphere level in a directory where process_smb had already completed. process_smb.log contains this:

compass calling: compass.landice.tests.ismip7_forcing.atmosphere.process_smb.ProcessSmb.runtime_setup()
  inherited from: compass.step.Step.runtime_setup()
  in /global/cfs/cdirs/m4288/users/trhille/compass/compass/step.py


compass calling: compass.landice.tests.ismip7_forcing.atmosphere.process_smb.ProcessSmb.run()
  in /global/cfs/cdirs/m4288/users/trhille/compass/compass/landice/tests/ismip7_forcing/atmosphere/process_smb.py

Using atmosphere source /global/cfs/cdirs/m4288/users/trhille/ISMIP7/forcing/GIS/CESM2-WACCM/ctrl/SDBN1-1000m/acabf/v3
Found 286 SMB files for years 2000-2301
Output already exists, skipping: /global/cfs/cdirs/m4288/users/trhille/ISMIP7/process_forcing_20260929/GIS/CESM2-WACCM_ctrl/atmosphere/GIS_1to10km_r03_20260925_SMB_CESM2-WACCM_ctrl_2000-2301.nc

The compass.o* file reads:

landice/ismip7_forcing/atmosphere
compass calling: compass.landice.tests.ismip7_forcing.atmosphere.Atmosphere.run()
  inherited from: compass.testcase.TestCase.run()
  in /global/cfs/cdirs/m4288/users/trhille/compass/compass/testcase.py

compass calling: compass.run.serial._run_test()
  in /global/cfs/cdirs/m4288/users/trhille/compass/compass/run/serial.py

Running steps:
  build_mapping_file
  process_smb
  process_temperature
  process_smb_gradient
  process_temperature_gradient
  process_runoff

  * step: build_mapping_file
  * step: process_smb
  * step: process_temperature

So, this is cleanly skipping the completed step and moving on to the next.

I have also confirmed that the bug fix in landice/ismip7/archive.py fixes an error I was encountering, And I have run the AIS fracture test case successfully.

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved moderate findings affect cache identity, partial-output handling, and 3-D restart detection.

Review effort: Lite
Findings: 1

Open (1)

3-D completion guard misses per-chunk output files · New

What changed in this PR

This PR adds restart-aware skipping for ISMIP7 forcing outputs and fixes fracture-version configuration lookup.

Changes:

Reuses completed atmosphere, ocean, and fracture forcing outputs.
Adds a 3-D thermal forcing skip check.
Corrects fracture configuration parsing.

File	Review summary
`compass/landice/tests/ismip7_forcing/ocean_thermal/process_thermal_forcing.py`	Four unresolved moderate findings (1 vote each): cache identities omit remapping or archive inputs, and direct copies can leave partial outputs.
`compass/landice/tests/ismip7_forcing/ocean_thermal/build_3d_thermal_forcing.py`	One unresolved moderate finding (4 votes): the checked path is not produced; all chunk outputs should be checked.
`compass/landice/tests/ismip7_forcing/fracture/process_shelf_collapse.py`	Two unresolved moderate findings (1 vote each): cache identity omits archive/remapping inputs, and existing destinations may be partial copies.
`compass/landice/tests/ismip7_forcing/fracture/process_lake_properties.py`	Two unresolved moderate findings (1 vote each): cache identity omits archive/remapping inputs, and existing destinations may be partial copies.
`compass/landice/tests/ismip7_forcing/fracture/process_excess_melt.py`	Two unresolved moderate findings (1 vote each): cache identity omits archive/remapping inputs, and existing destinations may be partial copies.
`compass/landice/tests/ismip7_forcing/atmosphere/process_temperature.py`	Two unresolved moderate findings (1 vote each): cache identity omits version/remapping inputs, and existing destinations may be partial copies.
`compass/landice/tests/ismip7_forcing/atmosphere/process_temperature_gradient.py`	Two unresolved moderate findings (1 vote each): cache identity omits archive/remapping inputs, and existing destinations may be partial copies.
`compass/landice/tests/ismip7_forcing/atmosphere/process_smb.py`	Two unresolved moderate findings (1 vote each): cache identity omits archive/remapping inputs, and existing destinations may be partial copies.
`compass/landice/tests/ismip7_forcing/atmosphere/process_smb_gradient.py`	Two unresolved moderate findings (1 vote each): cache identity omits archive/remapping inputs, and existing destinations may be partial copies.
`compass/landice/tests/ismip7_forcing/atmosphere/process_runoff.py`	Two unresolved moderate findings (1 vote each): cache identity omits archive/remapping inputs, and existing destinations may be partial copies.
`compass/landice/ismip7/archive.py`	Corrects fracture-version configuration lookup.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

The greenland_3d.run() function writes per-chunk output files named by their start year (e.g., GrIS_..._3dThermalForcing_OCX_2007.nc, ..._2017.nc), not a single file with the full year range. The previous guard checked for the non-existent full-range filename, so it never skipped even after a successful run. This commit fixes the guard to check for the first chunk file (with start_year only, not start_year-end_year), which greenland_3d.run actually produces. The check is conservative: it only verifies the first chunk exists, consistent with the per-year remapped file skip pattern used in the other ismip7_forcing steps. Addresses Copilot review comment on PR MPAS-Dev#1001. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

When an ismip7_forcing job times out mid-remap, it can leave truncated or incomplete remapped_* and extrap_* files. The existing per-file resume logic reused these based purely on os.path.exists(), silently feeding bad intermediates into the final concatenation, which then failed with 'ValueError: coordinate time not present in all datasets'. This commit adds netcdf_file_is_valid() to compass/landice/ismip7/remap.py, which checks that a file exists, opens cleanly, contains the expected variable and (optionally) time coordinate, and can actually read its last time record (catches header-complete but data-truncated files that open cleanly but fail later). Each step's per-file skip guard is now: 'file exists AND valid → skip; otherwise delete and regenerate.' Coverage: - State 1: file never created → os.path.exists catches it - State 2: truncated header (bad magic bytes) → raises on open - State 3: header OK but variable/time missing → presence check - State 4: header complete but data truncated → last-record .load() raises (this closes the subtle gap where lazy xarray reads only the header and the truncated file passes an open+presence check) Changes: - Add netcdf_file_is_valid() helper to remap.py - Update all 5 atmosphere steps (SMB, temperature, runoff, gradients) to validate both remapped_* (require_time=True) and extrap_* files - Update ocean_thermal/_process_ocean_forcing (tf, require_time=True) and _run_climatology (tf, require_time=False — static 3-D field) - Update fracture/process_shelf_collapse (mask, require_time=True) No change to excess_melt / lake_properties — those regenerate intermediates unconditionally (no exists-guard to reuse a stale file). Fixes the reported 'time not present' concat error and covers all four interrupted-write states at negligible cost (single-slice read). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Copilot

Copilot review overview

🟡 Changes recommended

Critical and moderate restart-integrity issues remain unresolved.

Review effort: Lite
Findings: 2 · 1 · 1

Open (4)

Detect source version changes before reusing outputs · New
Verify all expected chunks before skipping generation · New
Honor overwrite configuration before skipping existing chunks · New
Document the restart-validation helper in the developer API · New

Resolved since last review (1)

3-D completion guard misses per-chunk output files

Previously missed (10)

In code that hasn't changed since last review