Refactor `ismip7_forcing` by trhille · Pull Request #997 · MPAS-Dev/compass

trhille · 2026-09-24T22:26:44Z

Resolve forcing files directly from the native AIS and GIS archive layouts instead of requiring manually rearranged inputs.

centralize forcing path discovery in a shared archive resolver
select the latest available version independently for each dataset
support explicit version overrides and numeric version ordering
make forcing product and resolution configurable
support ESM, OCX, and fracture-data directory layouts
include source-grid information in mapping filenames
remove hard-coded versions and resolutions from ice-sheet parameters
update configuration, documentation, and resolver tests

Co-authored-by: Codex codex@openai.com

Checklist

User's Guide has been updated
Developer's Guide has been updated
API documentation in the Developer's Guide (api.rst) has any new or modified class, method and/or functions listed
Documentation has been built locally and changes look as expected
Document (in a comment titled Testing in this PR) any testing that was used to verify the changes

Resolve forcing files directly from the native AIS and GIS archive layouts instead of requiring manually rearranged inputs. * centralize forcing path discovery in a shared archive resolver * select the latest available version independently for each dataset * support explicit version overrides and numeric version ordering * make forcing product and resolution configurable * support ESM, OCX, and fracture-data directory layouts * include source-grid information in mapping filenames * remove hard-coded versions and resolutions from ice-sheet parameters * update configuration, documentation, and resolver tests Co-authored-by: Codex [codex@openai.com](mailto:codex@openai.com)

Add dedicated mapping steps for atmospheric, ocean, and fracture forcings. Reuse existing compatible mapping files and pass mapping outputs to the processing steps instead of generating them on demand. Update configuration, documentation, and tests for the new workflow.

Set min_tasks to 1 to prevent jobs from failing when the number of processors was reduced between running build_mapping_files and the remaining processing steps.

Add ntasks to mapping step test, which was failing during CI.

trhille · 2026-09-25T15:25:28Z

Testing

I used this to process OCX atmosphere and ocean thermal forcing on the 2-20km AIS mesh using the .cfg file below.

click to expand

# Example config for processing the AIS ISMIP7 OCX (reanalysis) forcing.
# OCX has no distinct ESM model: it uses RACMO2.3p2-ERA for the atmosphere and
# reanalysis ocean products, selected automatically when scenario = OCX. The
# [ismip7] model option is ignored for OCX (set it to None). Unlike GrIS, the
# AIS OCX ocean provides several forcing choices (main/cold/warm/vary) selected
# with the ocean_choice option below. The same config file drives both test
# cases; set them up individually, e.g.:
#   compass setup -t landice/ismip7_forcing/atmosphere -w WORKDIR -f ismip7_forcing_ocx_ais.cfg
#   compass setup -t landice/ismip7_forcing/ocean_thermal -w WORKDIR -f ismip7_forcing_ocx_ais.cfg

# config options for ismip7 forcing data
[ismip7]

# Ice sheet: ais (Antarctic) or gis (Greenland)
ice_sheet = ais

# Root of the native ISMIP7 archive (the directory containing AIS/ and GIS/).
base_path_ismip7 = /global/cfs/cdirs/m4288/users/trhille/ISMIP7/forcing

# Base path to the MALI mesh. User has to supply.
base_path_mali = /global/cfs/cdirs/fanssie/MALI_input_files/AIS_2to20km_r03

# Base path to which output forcing files are saved.
output_base_path = /global/cfs/cdirs/m4288/users/trhille/ISMIP7/test_COMPASS_refactor_PR997/AIS

# OCX has no distinct model (RACMO for atmosphere, reanalysis for ocean are
# selected automatically), so this option is ignored.
model = None

# Scenario for forcing data.
scenario = OCX

# Name of the MALI mesh. Used to name mapping and output files.
mali_mesh_name = AIS_2to20km_r03_20260922

# MALI mesh file. User has to supply.
mali_mesh_file = AIS_2to20km_r03_20260922.nc

# Number of MPI tasks for ESMF_RegridWeightGen
esmf_ntasks = 2048

# Whether to process time-varying ocean thermal forcing (ESM scenario data)
process_ocean_thermal = true

# Whether to process observational ocean thermal forcing climatology
process_ocean_climatology = false

# config options for ismip7 atmosphere forcing
[ismip7_atmosphere]

product = auto
resolution = auto
version = latest

# Remapping method. Options: bilinear, neareststod, conserve
method_remap = conserve

# Start year for processing
start_year = 2000

# End year for processing
end_year = 2025

# config options for ismip7 ocean thermal forcing
[ismip7_ocean_thermal]

resolution = auto
version = latest

# Remapping method. Options: bilinear, neareststod, conserve
method_remap = bilinear

# Ocean forcing choice(s) for the AIS OCX scenario. Comma-separated list of any
# of: main, cold, warm, vary. Use 'all' to process every choice. Each choice is
# written to its own OCX_<choice> output directory.
ocean_choice = main

# Start year for processing
start_year = 2000

# End year for processing
end_year = 2025

# config options for ismip7 ocean thermal forcing climatology
[ismip7_ocean_climatology]

# Remapping method. Options: bilinear, neareststod, conserve
method_remap = bilinear

version = latest

# Base path to observational climatology data
base_path_climatology = None

I was able to run the build_mapping_file step separately on 16 nodes (8 nodes ran into OOM), and then switch to 1 node to run the remaining processing steps. I also confirmed that running ocean_thermal multiple times did not create the circular symlink issue mentioned in #992.

Copilot

Copilot review overview

🟡 Changes recommended

The all-disabled fracture configuration still attempts source resolution, and several API and OCX documentation details need correction.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 · 3

Open (4)

Skip fracture source resolution when all methods are disabled · New
Scope the EN4 comment to GrIS · New
Add BuildMappingFile classes to autosummary · New
Clarify EN4 and AIS ocean source configurations · New

What changed in this PR

Refactors ISMIP7 forcing to resolve native archive layouts and generate reusable mapping files.

Changes:

Adds configurable archive, version, product, and resolution resolution.
Introduces dedicated mapping-generation and caching steps.
Updates processing, tests, configuration, and documentation.

Validation was limited to static review.

File	Description
`tests/landice/ismip7/test_mapping_step.py`	Tests mapping caching and fracture grid selection.
`tests/landice/ismip7/test_archive.py`	Tests archive resolution behavior.
`docs/users_guide/landice/test_groups/ismip7_forcing.rst`	Documents native layouts and mapping steps.
`docs/developers_guide/landice/test_groups/ismip7_forcing.rst`	Updates implementation documentation.
`docs/developers_guide/landice/framework.rst`	Documents the archive resolver.
`docs/developers_guide/landice/api.rst`	Adds archive APIs.
`compass/landice/tests/ismip7_forcing/ocean_thermal/process_thermal_forcing.py`	Uses resolved ocean sources and mappings.
`compass/landice/tests/ismip7_forcing/ocean_thermal/build_mapping_file.py`	Adds ocean mapping generation.
`compass/landice/tests/ismip7_forcing/ocean_thermal/__init__.py`	Registers the ocean mapping step.
`compass/landice/tests/ismip7_forcing/mapping_step.py`	Implements shared mapping caching.
`compass/landice/tests/ismip7_forcing/ismip7_forcing.cfg`	Adds resolver configuration defaults.
`compass/landice/tests/ismip7_forcing/ismip7_forcing_test.cfg`	Updates the example archive configuration.
`compass/landice/tests/ismip7_forcing/ismip7_forcing_ocx_gis.cfg`	Updates GrIS OCX configuration.
`compass/landice/tests/ismip7_forcing/ismip7_forcing_ocx_ais.cfg`	Updates AIS OCX configuration.
`compass/landice/tests/ismip7_forcing/fracture/process_shelf_collapse.py`	Uses resolved fracture inputs.
`compass/landice/tests/ismip7_forcing/fracture/process_lake_properties.py`	Uses resolved fracture inputs.
`compass/landice/tests/ismip7_forcing/fracture/process_excess_melt.py`	Uses resolved fracture inputs.
`compass/landice/tests/ismip7_forcing/fracture/build_mapping_file.py`	Adds fracture mapping generation.
`compass/landice/tests/ismip7_forcing/fracture/__init__.py`	Registers the fracture mapping step.
`compass/landice/tests/ismip7_forcing/atmosphere/process_temperature.py`	Uses resolved atmosphere inputs.
`compass/landice/tests/ismip7_forcing/atmosphere/process_temperature_gradient.py`	Uses resolved atmosphere inputs.
`compass/landice/tests/ismip7_forcing/atmosphere/process_smb.py`	Uses resolved atmosphere inputs.
`compass/landice/tests/ismip7_forcing/atmosphere/process_smb_gradient.py`	Uses resolved atmosphere inputs.
`compass/landice/tests/ismip7_forcing/atmosphere/process_runoff.py`	Uses resolved atmosphere inputs.
`compass/landice/tests/ismip7_forcing/atmosphere/build_mapping_file.py`	Adds atmosphere mapping generation.
`compass/landice/tests/ismip7_forcing/atmosphere/__init__.py`	Registers the atmosphere mapping step.
`compass/landice/ismip7/ice_sheet_params.py`	Removes fixed versions and resolutions.
`compass/landice/ismip7/archive.py`	Implements native archive resolution.

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

Add check to skip fracture mappings if none are requested. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

When fracture method config options are not set, section.get() returns None instead of a string. Check for None before calling .lower() to avoid AttributeError and properly skip fracture mappings when no methods are configured. Addresses review comment on PR MPAS-Dev#997.

Update comment to specify that EN4 ocean forcing is used for GrIS in the OCX scenario, not AIS. AIS OCX deliberately leaves ocean_model unset and uses per-choice ocean directories (main/cold/warm/vary). Addresses review comment on PR MPAS-Dev#997.

Add the three new BuildMappingFile classes (atmosphere, ocean_thermal, and fracture) to the ismip7_forcing autosummary in the API documentation so their API pages will be generated. Addresses review comment on PR MPAS-Dev#997.

Update documentation to distinguish that GrIS OCX uses EN4 as the ocean source, while AIS OCX leaves ocean_model unset and uses per-choice ocean directories (main/cold/warm/vary). Addresses review comment on PR MPAS-Dev#997.

Break long line into multiple lines to satisfy 79 character limit. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

matthewhoffman

@trhille , I manually skimmed all the changes, looked over copilot's review, used copilot and claude to fix copilot review issues, and compiled and manually looked over docs. I'm approving based on these steps, plus your Testing comment demonstrating this behaves as intended.

Replace manual get_params/forcing_group/source logic with resolve_ocean_source, so the step uses the same label and forcing_group that ProcessThermalForcing wrote. The 2D/3D filename contract now matches what process_thermal_forcing produces (mesh_<2d/3d>ThermalForcing_<label>_). This is a semantic-conflict resolution: build_3d originally mirrored pre-MPAS-Dev#997 source logic; now it consumes the MPAS-Dev#997 ForcingSource object. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

trhille force-pushed the landice/refactor_ismip7_forcing branch 2 times, most recently from bd81e5d to 3b49480 Compare September 25, 2026 05:20

trhille added 3 commits September 24, 2026 22:28

Fix bug in config parser calls

436dbc9

trhille force-pushed the landice/refactor_ismip7_forcing branch from 3b49480 to 436dbc9 Compare September 25, 2026 05:31

trhille changed the title ~~Refactor ISMIP7 forcing archive resolution~~ Refactor ismip7_forcing Sep 25, 2026

trhille added 3 commits September 24, 2026 22:37

Fix bug in determining ocean version for file naming

4ee9a91

Allow different numbers of processors between steps

d7c22d2

Set min_tasks to 1 to prevent jobs from failing when the number of processors was reduced between running build_mapping_files and the remaining processing steps.

Add ntasks to mapping step test

fc9923e

Add ntasks to mapping step test, which was failing during CI.

This was referenced Sep 25, 2026

Split ESMF weight generation into dedicated steps for ISMIP7 forcing #994

Closed

Prevent circular symlinks from overwriting atmosphere files #992

Closed

trhille requested a review from hollyhan September 25, 2026 14:20

trhille added this to the ISMIP7 milestone Sep 25, 2026

matthewhoffman requested a balanced review from Copilot September 25, 2026 17:29

Copilot started reviewing on behalf of matthewhoffman September 25, 2026 17:29 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Comment thread compass/landice/tests/ismip7_forcing/fracture/build_mapping_file.py

Comment thread compass/landice/ismip7/ice_sheet_params.py Outdated

Comment thread docs/developers_guide/landice/api.rst

Comment thread docs/developers_guide/landice/test_groups/ismip7_forcing.rst Outdated

matthewhoffman and others added 6 commits September 25, 2026 12:47

Skip fracture mappings if no methods are specified

465ede5

Add check to skip fracture mappings if none are requested. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Clarify EN4 is only used for GrIS OCX

0f94381

Update comment to specify that EN4 ocean forcing is used for GrIS in the OCX scenario, not AIS. AIS OCX deliberately leaves ocean_model unset and uses per-choice ocean directories (main/cold/warm/vary). Addresses review comment on PR MPAS-Dev#997.

Add BuildMappingFile classes to API documentation

539ecf3

Add the three new BuildMappingFile classes (atmosphere, ocean_thermal, and fracture) to the ismip7_forcing autosummary in the API documentation so their API pages will be generated. Addresses review comment on PR MPAS-Dev#997.

Clarify OCX ocean source differs between GrIS and AIS

4b200c4

Update documentation to distinguish that GrIS OCX uses EN4 as the ocean source, while AIS OCX leaves ocean_model unset and uses per-choice ocean directories (main/cold/warm/vary). Addresses review comment on PR MPAS-Dev#997.

Fix flake8 line length error in fracture build_mapping_file

04db5d8

Break long line into multiple lines to satisfy 79 character limit. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

matthewhoffman force-pushed the landice/refactor_ismip7_forcing branch from 0264f24 to 04db5d8 Compare September 25, 2026 19:33

matthewhoffman approved these changes Sep 25, 2026

View reviewed changes

matthewhoffman merged commit ffc7196 into MPAS-Dev:main Sep 25, 2026
5 checks passed

trhille mentioned this pull request Sep 26, 2026

Update streams and namelist options for ismip7_run #993

Merged

1 task

Navigation Menu

Refactor `ismip7_forcing` - #997

Refactor `ismip7_forcing` #997
matthewhoffman merged 12 commits into
MPAS-Dev:mainfrom
trhille:landice/refactor_ismip7_forcing

trhille commented Sep 24, 2026 •

edited by matthewhoffman

Loading

Uh oh!

trhille commented Sep 25, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matthewhoffman left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

trhille commented Sep 24, 2026 • edited by matthewhoffman Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

trhille commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matthewhoffman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

trhille commented Sep 24, 2026 •

edited by matthewhoffman

Loading

trhille commented Sep 25, 2026 •

edited

Loading