Skip to content

Skip ismip7_forcing steps when final output already exists #1001

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
trhille merged 6 commits into from
Oct 1, 2026
+287 −102
Merged

Skip ismip7_forcing steps when final output already exists #1001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4eeada9
Skip ismip7_forcing steps when final output already exists
trhille Sep 29, 2026
30c3cad
Fix bug in config parser when determining fracture forcing version
trhille Sep 29, 2026
90fc71a
Fix 3-D thermal forcing completion check to use actual chunk filename
trhille Sep 29, 2026
9994e92
Validate and reprocess incomplete intermediate files on resume
trhille Sep 29, 2026
aa20937
Embed source version in output filenames; remove conflicting 3-D guar…
trhille Sep 29, 2026
fd5ee59
Fix 3-D resume and decode_times crash from PR review
trhille Oct 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions compass/landice/ismip7/archive.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ def resolve_fracture_source(config, file_pattern):
scenario = config.get('ismip7', 'scenario')
base_path = os.path.join(
_ice_sheet_path(config), model, scenario, 'fracture')
requested_version = config.get(
'ismip7_fracture', 'version', fallback='latest')
requested_version = config['ismip7_fracture'].get(
'version', fallback='latest')
version_path, version, files = resolve_version_directory(
base_path, requested_version, file_pattern)

Expand Down
54 changes: 54 additions & 0 deletions compass/landice/ismip7/remap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,60 @@
from scipy.ndimage import distance_transform_edt


def netcdf_file_is_valid(path, varname=None, require_time=False,
logger=None):
Comment on lines +13 to +14

@trhille trhille Sep 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in aa20937

"""
Return True if ``path`` exists, opens cleanly, contains ``varname`` and
(optionally) a ``time`` coordinate, and its last time record can
actually be read. Used to detect incomplete intermediate files left by
an interrupted (timed-out) run so they can be regenerated instead of
silently reused. The last-record read catches a file whose header is
complete but whose data section was truncated mid-write (which opens
cleanly but fails later at concatenation).

Parameters
----------
path : str
Path to the NetCDF file to check

varname : str, optional
Variable name to verify is present; if None, only checks file opens

require_time : bool, optional
Whether to require a ``time`` coordinate/variable

logger : logging.Logger, optional
Logger for status messages (logs incomplete-file info if provided)

Returns
-------
bool
True if the file is valid and complete; False otherwise
"""
if not os.path.exists(path):
return False
try:
with xr.open_dataset(path, decode_times=False) as ds:
if varname is not None and varname not in ds.variables:
return False
if require_time and "time" not in ds.variables:
return False
# Force-read the last record to detect a truncated data
# section (header OK, but bytes cut off mid-file).
if varname is not None:
var = ds[varname]
if "time" in var.dims:
var.isel(time=-1).load()
else:
var.load()
except Exception as exc:
if logger is not None:
logger.info(f" Ignoring incomplete/unreadable file "
f"{os.path.basename(path)}: {exc}")
return False
return True


def extrapolate_source(input_file, output_file, varnames, logger):
"""
Extrapolate fill/missing values on the source polar stereographic grid
Expand Down
38 changes: 27 additions & 11 deletions compass/landice/tests/ismip7_forcing/atmosphere/process_runoff.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
mapping_file_name,
resolve_atmosphere_source,
)
from compass.landice.ismip7.remap import extrapolate_source
from compass.landice.ismip7.remap import (
extrapolate_source,
netcdf_file_is_valid,
)
from compass.step import Step


Expand Down Expand Up @@ -93,6 +96,16 @@ def run(self):
logger.info(f"Found {len(input_files)} runoff files for years "
f"{start_year}-{end_year}")

# Check if final output already exists; skip if so
output_file = (f"{mali_mesh_name}_runoff_{model}_{scenario}_"
f"{source.version}_{start_year}-{end_year}.nc")
output_path = os.path.join(output_base_path, forcing_group,
"atmosphere")
dst = os.path.join(output_path, output_file)
if os.path.exists(dst):
logger.info(f"Output already exists, skipping: {dst}")
return

# The mapping file is supplied by the build_mapping_file step.
mapping_file = mapping_file_name(
config, "atm", source.source_grid, method_remap)
Expand All @@ -104,16 +117,25 @@ def run(self):
remapped_file = f"remapped_{basename}"
remapped_files.append(remapped_file)

if os.path.exists(remapped_file):
if netcdf_file_is_valid(remapped_file, "mrro",
require_time=True, logger=logger):
logger.info(f" Remapped file exists, skipping: {basename}")
continue
# Incomplete remapped file from an interrupted run: drop and
# redo
if os.path.exists(remapped_file):
logger.info(f" Reprocessing incomplete remapped file: "
f"{basename}")
os.remove(remapped_file)

# Extrapolate fill values on source grid before remapping
# so they don't pollute neighboring cells during interpolation
extrap_file = f"extrap_{basename}"
if not os.path.exists(extrap_file):
extrapolate_source(input_file, extrap_file, "mrro",
logger)
if not netcdf_file_is_valid(extrap_file, "mrro",
logger=logger):
if os.path.exists(extrap_file):
os.remove(extrap_file)
extrapolate_source(input_file, extrap_file, "mrro", logger)

logger.info(f" Remapping: {basename}")
args = ["ncremap",
Expand All @@ -129,9 +151,6 @@ def run(self):

# Combine remapped files and rename to MALI conventions
logger.info("Combining remapped files and renaming variables...")
output_file = (f"{mali_mesh_name}_runoff_{model}_{scenario}_"
f"{start_year}-{end_year}.nc")

self._combine_and_rename(remapped_files, output_file)

# Clean up remapped files
Expand All @@ -141,12 +160,9 @@ def run(self):
os.remove(f)

# Place output in appropriate directory
output_path = os.path.join(output_base_path, forcing_group,
"atmosphere")
if not os.path.exists(output_path):
os.makedirs(output_path)

dst = os.path.join(output_path, output_file)
shutil.copy(output_file, dst)

logger.info(f"Done. Output: {dst}")
Expand Down
35 changes: 26 additions & 9 deletions compass/landice/tests/ismip7_forcing/atmosphere/process_smb.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
mapping_file_name,
resolve_atmosphere_source,
)
from compass.landice.ismip7.remap import extrapolate_source
from compass.landice.ismip7.remap import (
extrapolate_source,
netcdf_file_is_valid,
)
from compass.step import Step


Expand Down Expand Up @@ -93,6 +96,16 @@ def run(self):
logger.info(f"Found {len(input_files)} SMB files for years "
f"{start_year}-{end_year}")

# Check if final output already exists; skip if so
output_file = (f"{mali_mesh_name}_SMB_{model}_{scenario}_"
f"{source.version}_{start_year}-{end_year}.nc")

@xylar xylar Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the version in the name, a new version writes a second file next to the old one, but ismip7_run expects exactly one of each. When I went from v1 to v2, its setup exited for SMB, temperature and shelf collapse, and gave runoff and the gradients an empty filename. Output written before this commit also won't be recognized, so a resume reprocesses everything and leaves both files. I'd either take the version out of the filenames (and delete outputs to regenerate) or have each step remove other versions' files when it writes. This applies to all the steps.

@trhille trhille Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"When I went from v1 to v2, its setup exited for SMB, temperature and shelf collapse, and gave runoff and the gradients an empty filename." Was this for ismip7_run? I'm not sure I follow what happened with runoff and gradients. Is this a bug in ismip7_run?

"Output written before this commit also won't be recognized, so a resume reprocesses everything and leaves both files." I don't think I care about backward compatibility before this commit, since we can easily manually rename previously processed files if necessary to avoid reprocessing. However, I'll think a little more about it.

@xylar xylar Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is ismip7_run's set_up_experiment.py. It exits unless it finds exactly one SMB, temperature, 3-D TF or shelf-collapse file. For runoff and the gradients, though, it just leaves the filename empty, so those streams get filename_template="". I didn't check what MALI does with that.

The gap is in ismip7_run, but this PR is what triggers it. Before, a rerun after an archive version bump overwrote the output; now it adds a second file next to the old one. If you'd rather clean up by hand when that happens, that's fine, but then ismip7_run should error on extra runoff and gradient files too.

Agreed on outputs from before this commit; renaming by hand is fine.

Posted by Claude Code on @xylar's behalf. The testing, analysis and wording above are AI-authored; please check them accordingly.

@xylar xylar Oct 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trhille, if this is just Claude misusing the tool, feel free to ignore this advice.

output_path = os.path.join(output_base_path, forcing_group,
"atmosphere")
dst = os.path.join(output_path, output_file)
if os.path.exists(dst):
logger.info(f"Output already exists, skipping: {dst}")
return

# The mapping file is supplied by the build_mapping_file step.
mapping_file = mapping_file_name(
config, "atm", source.source_grid, method_remap)
Expand All @@ -104,14 +117,24 @@ def run(self):
remapped_file = f"remapped_{basename}"
remapped_files.append(remapped_file)

if os.path.exists(remapped_file):
if netcdf_file_is_valid(remapped_file, "acabf",
require_time=True, logger=logger):
logger.info(f" Remapped file exists, skipping: {basename}")
continue
# Incomplete remapped file from an interrupted run: drop and
# redo
if os.path.exists(remapped_file):
logger.info(f" Reprocessing incomplete remapped file: "
f"{basename}")
os.remove(remapped_file)

# Extrapolate fill values on source grid before remapping
# so they don't pollute neighboring cells during interpolation
extrap_file = f"extrap_{basename}"
if not os.path.exists(extrap_file):
if not netcdf_file_is_valid(extrap_file, "acabf",
logger=logger):
if os.path.exists(extrap_file):
os.remove(extrap_file)
extrapolate_source(input_file, extrap_file, "acabf", logger)

logger.info(f" Remapping: {basename}")
Expand All @@ -128,9 +151,6 @@ def run(self):

# Combine remapped files and rename to MALI conventions
logger.info("Combining remapped files and renaming variables...")
output_file = (f"{mali_mesh_name}_SMB_{model}_{scenario}_"
f"{start_year}-{end_year}.nc")

self._combine_and_rename(remapped_files, output_file)

# Clean up remapped files
Expand All @@ -140,12 +160,9 @@ def run(self):
os.remove(f)

# Place output in appropriate directory
output_path = os.path.join(output_base_path, forcing_group,
"atmosphere")
if not os.path.exists(output_path):
os.makedirs(output_path)

dst = os.path.join(output_path, output_file)
shutil.copy(output_file, dst)

logger.info(f"Done. Output: {dst}")
Expand Down
35 changes: 26 additions & 9 deletions compass/landice/tests/ismip7_forcing/atmosphere/process_smb_gradient.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
mapping_file_name,
resolve_atmosphere_source,
)
from compass.landice.ismip7.remap import extrapolate_source
from compass.landice.ismip7.remap import (
extrapolate_source,
netcdf_file_is_valid,
)
from compass.step import Step


Expand Down Expand Up @@ -94,6 +97,16 @@ def run(self):
logger.info(f"Found {len(input_files)} SMB gradient files for years "
f"{start_year}-{end_year}")

# Check if final output already exists; skip if so
output_file = (f"{mali_mesh_name}_SMB_gradient_{model}_{scenario}_"
f"{source.version}_{start_year}-{end_year}.nc")
output_path = os.path.join(output_base_path, forcing_group,
"atmosphere")
dst = os.path.join(output_path, output_file)
if os.path.exists(dst):
logger.info(f"Output already exists, skipping: {dst}")
return

# The mapping file is supplied by the build_mapping_file step.
mapping_file = mapping_file_name(
config, "atm", source.source_grid, method_remap)
Expand All @@ -105,14 +118,24 @@ def run(self):
remapped_file = f"remapped_{basename}"
remapped_files.append(remapped_file)

if os.path.exists(remapped_file):
if netcdf_file_is_valid(remapped_file, "dacabfdz",
require_time=True, logger=logger):
logger.info(f" Remapped file exists, skipping: {basename}")
continue
# Incomplete remapped file from an interrupted run: drop and
# redo
if os.path.exists(remapped_file):
logger.info(f" Reprocessing incomplete remapped file: "
f"{basename}")
os.remove(remapped_file)

# Extrapolate fill values on source grid before remapping
# so they don't pollute neighboring cells during interpolation
extrap_file = f"extrap_{basename}"
if not os.path.exists(extrap_file):
if not netcdf_file_is_valid(extrap_file, "dacabfdz",
logger=logger):
if os.path.exists(extrap_file):
os.remove(extrap_file)
extrapolate_source(input_file, extrap_file, "dacabfdz",
logger)

Expand All @@ -130,9 +153,6 @@ def run(self):

# Combine remapped files and rename to MALI conventions
logger.info("Combining remapped files and renaming variables...")
output_file = (f"{mali_mesh_name}_SMB_gradient_{model}_{scenario}_"
f"{start_year}-{end_year}.nc")

self._combine_and_rename(remapped_files, output_file)

# Clean up remapped files
Expand All @@ -142,12 +162,9 @@ def run(self):
os.remove(f)

# Place output in appropriate directory
output_path = os.path.join(output_base_path, forcing_group,
"atmosphere")
if not os.path.exists(output_path):
os.makedirs(output_path)

dst = os.path.join(output_path, output_file)
shutil.copy(output_file, dst)

logger.info(f"Done. Output: {dst}")
Expand Down
34 changes: 25 additions & 9 deletions compass/landice/tests/ismip7_forcing/atmosphere/process_temperature.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
mapping_file_name,
resolve_atmosphere_source,
)
from compass.landice.ismip7.remap import extrapolate_source
from compass.landice.ismip7.remap import (
extrapolate_source,
netcdf_file_is_valid,
)
from compass.step import Step


Expand Down Expand Up @@ -93,6 +96,16 @@ def run(self):
logger.info(f"Found {len(input_files)} temperature files for years "
f"{start_year}-{end_year}")

# Check if final output already exists; skip if so
output_file = (f"{mali_mesh_name}_temperature_{model}_{scenario}_"
f"{source.version}_{start_year}-{end_year}.nc")
output_path = os.path.join(output_base_path, forcing_group,
"atmosphere")
dst = os.path.join(output_path, output_file)
if os.path.exists(dst):
logger.info(f"Output already exists, skipping: {dst}")
return
Comment on lines +105 to +107

@trhille trhille Sep 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in aa20937


# The mapping file is supplied by the build_mapping_file step.
mapping_file = mapping_file_name(
config, "atm", source.source_grid, method_remap)
Expand All @@ -104,14 +117,23 @@ def run(self):
remapped_file = f"remapped_{basename}"
remapped_files.append(remapped_file)

if os.path.exists(remapped_file):
if netcdf_file_is_valid(remapped_file, "ts", require_time=True,
logger=logger):
logger.info(f" Remapped file exists, skipping: {basename}")
continue
# Incomplete remapped file from an interrupted run: drop and
# redo
if os.path.exists(remapped_file):
logger.info(f" Reprocessing incomplete remapped file: "
f"{basename}")
os.remove(remapped_file)

# Extrapolate fill values on source grid before remapping
# so they don't pollute neighboring cells during interpolation
extrap_file = f"extrap_{basename}"
if not os.path.exists(extrap_file):
if not netcdf_file_is_valid(extrap_file, "ts", logger=logger):
if os.path.exists(extrap_file):
os.remove(extrap_file)
extrapolate_source(input_file, extrap_file, "ts", logger)

logger.info(f" Remapping: {basename}")
Expand All @@ -128,9 +150,6 @@ def run(self):

# Combine remapped files and rename to MALI conventions
logger.info("Combining remapped files and renaming variables...")
output_file = (f"{mali_mesh_name}_temperature_{model}_{scenario}_"
f"{start_year}-{end_year}.nc")

self._combine_and_rename(remapped_files, output_file)

# Clean up remapped files
Expand All @@ -140,12 +159,9 @@ def run(self):
os.remove(f)

# Place output in appropriate directory
output_path = os.path.join(output_base_path, forcing_group,
"atmosphere")
if not os.path.exists(output_path):
os.makedirs(output_path)

dst = os.path.join(output_path, output_file)
shutil.copy(output_file, dst)

logger.info(f"Done. Output: {dst}")
Expand Down
Loading
Loading