Match layer/interface through the grid, not string substitution

[hdrake]

Fixes MOM6-community#51.

convergent_transport validated its layer/interface arguments by string substitution:

if layer.replace("l", "i") != interface:
    raise ValueError("Inconsistent layer and interface grid variables!")

str.replace rewrites every "l", so the check only worked for stems that contain no l. lam_l/lam_i and level_l/level_i were rejected outright — those models simply could not call convergent_transport with layer=/interface= — while unrelated pairs whose substitution coincided (ml_l/mi_i) were silently accepted.

Changes

• Ask the grid first. _validate_layer_interface accepts the pair if any axis of the grid registers layer at its "center" position and interface at an interface position of that same axis (outer/inner/left/right). That is the question the substitution was standing in for, and the grid already answers it for arbitrary names. It is also how callers construct the arguments in the first place — xwmb reads them off grid.axes["Z"].coords["center"]/["outer"] and hands them straight back.
• Fall back to a suffix-anchored convention when the grid registers neither name. This matters in practice: grids handed to sectionate usually declare only their horizontal axes, since sections are only traced horizontally — examples/load_example_model_grid.py registers X and Y only, and notebooks 2 and 3 then pass layer="sigma2_l", interface="sigma2_i". So the grid has nothing to say about that pair and a purely axis-based check would break the examples. The fallback accepts <stem>l/<stem>i anchored at the end of the name (layer[:-1] == interface[:-1]), which is what the substitution was reaching for; it covers z_l/z_i, sigma2_l/sigma2_i and lam_l/lam_i alike, and does not depend on what the stem spells.
• Error message names the arguments and the grid's axes, instead of "Inconsistent layer and interface grid variables!".
• Updated the interface docstring, which described the old substitution rule.

Behavior is unchanged for every pair that was previously accepted and is genuinely consistent; the only pairs that change from accept to reject are coincidental matches like ml_l/mi_i.

Tests

Added to sectionate/tests/test_convergent_transport.py, using a minimal 1x1-cell grid with a single-layer vertical coordinate:

• test_layer_interface_pairs_accepted — z_l/z_i, sigma2_l/sigma2_i, lam_l/lam_i, level_l/level_i, each with and without the vertical axis registered on the grid (8 cases); asserts both coordinates reach the output and that the transport is unchanged.
• test_layer_interface_from_grid_axis_without_naming_convention — MyCenters/MyEdges paired by the grid axis and following no _l/_i convention.
• test_inconsistent_layer_interface_rejected — z_l/sigma2_i, ml_l/mi_i (the old false accept), and z_l/z_l still raise ValueError.

These add 12 cases to the file (15 total). 8 of the 12 fail on the current branch and pass with the fix; the other 4 are the z_l/z_i and sigma2_l/sigma2_i pairs that already worked and must keep working.

Verification

Full suite in an env with xgcm 0.10.1: 73 passed, 0 skipped, 0 failed (data/ symlinked from an existing checkout, so the ECCO LLC90 and MOM6-fold tests ran rather than skipping).

I did not re-execute the example notebooks. Instead I checked the exact path they take directly, against the real CM4p25 data file: load_MOM6_example_grid() returns a grid registering only X and Y, and convergent_transport(..., layer="sigma2_l", interface="sigma2_i") on an OSNAP-like section returns as before, with both sigma2_l and sigma2_i on the output. Worth a notebook run before merge.

Separate issue noticed while testing

A grid that does register a vertical axis cannot be traced at all: build_neighbor_maps builds padding_width = {ax: (1, 1) for ax in grid.axes} and pads the 2-D corner arrays over every axis, so grid_section raises KeyError: "None of the DataArray's dims ('yq', 'xq') were found in axis coords." as soon as a Z axis exists. Unrelated to this change and left alone here — the new tests supply section indices directly rather than working around it — but it should probably get its own issue.

_{Drafted with AI assistance (Claude Code). I have read the diff and ran the tests and the reproducer myself.}

---

Update: take layer/interface from the grid's Z axis (second commit)

The check above stops at validating the caller's strings against the grid. If the grid can answer the question, though, the caller should not have to ask it. convergent_transport now reads the pair off the grid's "Z" axis when it has one, and both parameters default to None.

This is the same evidence as MOM6-community#51, followed one step further: xwmb/budget.py builds the arguments as

kwargs = {
    "layer":     self.grid.axes["Z"].coords['center'],
    "interface": self.grid.axes["Z"].coords['outer'],
}

— reads them off the axis, hands them back as strings, and sectionate re-derives the relationship. Deriving directly deletes the round trip.

Behavior

grid	layer=/interface= passed	result
has a Z axis	nothing	taken from the axis (center + outer/inner/left/right)
has a Z axis	names that agree with it	accepted, same result — this is the xwmb call pattern
has a Z axis	a name that contradicts it	ValueError
no Z axis	both names	validated against each other as before
no Z axis	nothing	no vertical coordinate on the output

A contradicting name raises rather than overriding. A caller who disagrees with the grid about which vertical coordinate its transports live on is confused, and quietly preferring one would label the output with a coordinate that need not describe the data — which is the same failure mode that produced MOM6-community#51 in the first place. The error names both the argument and what the axis says.

Only names that are actually present in grid._ds are derived: xgcm requires an axis position to name a dimension, but a dimension need not carry coordinate values, and these names are attached to the output by lookup.

The defaults change from "z_l"/"z_i" to None

This is a breaking change on paper. In practice the old default was unusable: it looks up grid._ds["z_l"], so on any grid without that variable it raised

KeyError: "No variable named 'z_l'. Variables on the dataset include ['xh', 'yh', 'xq', ...]"

I audited every convergent_transport call in the tree — 32 call sites (test_left_grid.py 7, test_section_multitile.py 9, test_convergent_transport.py 5, test_cube_left_grid.py 4, test_section_reversibility.py 2, examples 2/3/5). Every one passes layer= explicitly (26 as None, 4 as "sigma2_l", 2 as None through a splatted **kw). Zero rely on the implicit default — because it could not be relied on. So the change fixes a landmine rather than removing a working convenience, and nothing in the tree needed updating.

Worth a release note nonetheless, for any downstream caller on a genuine MOM6 z_l/z_i grid that omitted the arguments. Those callers now get output with no vertical coordinate rather than the z_l one — unless their grid registers a Z axis, in which case they get it back automatically.

Tests

7 more cases in test_convergent_transport.py: derivation from the axis; derivation identical to passing the same names explicitly (xr.testing.assert_identical); the three contradiction cases; no-axis-no-names attaching nothing; and a Z position naming a bare dimension with no coordinate values, which is not derived.

test_inconsistent_layer_interface_rejected now builds its grid with register_z=False. It tests the name-only validation path, and on a Z-registered grid a contradicting name is now caught earlier by the more specific check.

Verification

• Full suite: 80 passed, 0 skipped, 0 failed (61 on the base branch, +12 from the first commit, +7 here).
• All five example notebooks re-executed on this head — see the notebook section below.

I think this belongs here rather than on #8: it is transports.py, this PR's file, and #8 is gridutils.py and should stay single-purpose. Happy to move it if you would rather.

---

Example notebooks

Re-executed all five in a dedicated env built from docs/environment.yml with this branch pip install -e'd (xgcm 0.10.1, sectionate 0.4.0rc2.dev1), data symlinked from an existing checkout:

notebook	result
1_creating_an_OSNAP_section.ipynb	executed cleanly
2_OSNAP_transports_CM4p25.ipynb	executed cleanly
3_Labrador_convergence_CM4p25.ipynb	executed cleanly
4_sections_on_global_tripolar_grid.ipynb	executed cleanly
5_MOC_transports_ECCOv4r4.ipynb	executed cleanly

Outputs are unchanged apart from the version banner: comparing every textual output against the committed ones, the only differences are Sectionate version: 0.3.3 → 0.4.0rc2.dev1, the absent "Downloading … from Zenodo" lines (the data was already present), and warning messages carrying my paths. Notebook 5's overturning streamfunction reproduces bit-for-bit: psi range (Sv): -58.09992975038242 to 53.35870875408872, identical to the committed output.

The re-executed notebooks are committed (last commit on this branch). They were re-run fresh against this branch's current HEAD, so the committed outputs are the ones this branch's committed code actually produces. Two caveats are recorded in that commit message: the version banner reads 0.4.0rc2.dev1, a hatch-vcs dev string from an editable install of an untagged branch, and warning text cites build-worktree paths rather than a normal checkout. Both are artifacts of how the notebooks were run, not of this change, and both resolve on a refresh from a tagged release. See the merge-order note at the end.

_{This section drafted with AI assistance (Claude Code); I ran the notebooks and the comparisons myself.}

---

⚠️ Merge order: #7 and #8 now conflict on the notebooks

The re-executed notebooks are committed here, and the same is true on the other PR, so #7 and #8 no longer merge in either order without a conflict. I verified this rather than assuming it — a test merge of the two pushed refs conflicts on exactly the five notebooks:

examples/1_creating_an_OSNAP_section.ipynb
examples/2_OSNAP_transports_CM4p25.ipynb
examples/3_Labrador_convergence_CM4p25.ipynb
examples/4_sections_on_global_tripolar_grid.ipynb
examples/5_MOC_transports_ECCOv4r4.ipynb

Everything else merges cleanly: sectionate/transports.py, sectionate/gridutils.py and both test files are disjoint between the two branches. The conflict is entirely re-executed output, not code.

Suggested order: #8 first, then #7

• Pad only over the axes the array actually spans #8 is the smaller, self-contained fix (gridutils.py only) and its notebook refresh is a no-op by construction — every notebook grid registers only X and Y, so _pad_axes returns exactly ["X", "Y"], the same mapping the old code built. Its outputs differ from the current ones only in the version banner and warning paths.
• Match layer/interface through the grid, not string substitution #7 changes convergent_transport's API (the layer/interface defaults), so its notebook outputs are the ones with any semantic content. Merging it second means the surviving copy is the one produced by the later code.

Resolving the conflict

Don't hand-merge the notebook JSON. Take one side wholesale and, ideally, re-execute once on the merge result:

git checkout --theirs examples/   # after merging #8, take #7's notebooks
git add examples/
# then, best-effort:
cd examples && jupyter nbconvert --to notebook --execute --inplace \
  --ExecutePreprocessor.timeout=1800 --ExecutePreprocessor.kernel_name=python3 *.ipynb

Since neither branch changes any notebook result (verified: notebook 5's streamfunction is bit-identical on both, psi range (Sv): -58.09992975038242 to 53.35870875408872), taking either side is numerically safe; re-executing afterwards just makes the banner and warning paths consistent with the merged tree.

If you would rather not carry the churn at all, dropping the notebook commit from both branches (git revert of the last commit on each) restores clean either-order merging — which is what I had originally recommended. Either is fine; this note is just so nothing is a surprise at merge time.

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB