Skip to content

fix: correct misplaced parentheses in _Spatialslip zeta corrections#2743

Merged
erikvansebille merged 2 commits into
Parcels-code:mainfrom
steps-re:fix/spatialslip-zeta-parentheses
Jul 13, 2026
Merged

fix: correct misplaced parentheses in _Spatialslip zeta corrections#2743
erikvansebille merged 2 commits into
Parcels-code:mainfrom
steps-re:fix/spatialslip-zeta-parentheses

Conversation

@steps-re

Copy link
Copy Markdown
Contributor

What

In the 3D branch of _Spatialslip (src/parcels/interpolators/_xinterpolators.py), the four vertical (zeta) slip corrections have a misplaced closing parenthesis:

f_u = np.where(
    is_land(0, 0, 0, 0) & is_land(0, 0, 0, 1) & is_land(0, 0, 1, 0 & is_land(0, 0, 1, 1) & (zeta > 0)),
    f_u * (a + b * zeta) / zeta,
    f_u,
)

The ) after is_land(0, 0, 1, 0 is missing, so 0 & is_land(0, 0, 1, 1) & (zeta > 0) is passed as is_land's 4th positional argument (the x-index) instead of 0. The & is_land(...) & (zeta > 0) mask that should gate np.where is swallowed into that argument, so both the land test and the zeta position gate are wrong. This breaks the vertical free-/partial-slip boundary correction for 3D C-grid velocities.

Fix

Move the parenthesis so each line matches the analogous, correctly-formed eta/xsi corrections in the same function:

is_land(0, 0, 0, 0) & is_land(0, 0, 0, 1) & is_land(0, 0, 1, 0) & is_land(0, 0, 1, 1) & (zeta > 0)

Same correction applied to all four affected lines (the zeta > 0 and zeta < 1 gates for f_u and f_v).

Testing

pytest tests/test_interpolation.py tests/test_convert.py passes (29 passed, 1 xfailed) with the change. Note the cgrid_velocity case in test_interpolation.py is currently commented out, which is likely why this went unnoticed.

In the 3D branch of _Spatialslip, the four vertical (zeta) slip corrections
had a misplaced closing parenthesis, e.g.:

    is_land(0, 0, 1, 0 & is_land(0, 0, 1, 1) & (zeta > 0))

so the `& is_land(...) & (zeta > 0)` mask meant to gate np.where was instead
swallowed into is_land's 4th positional argument (the x-index). Both the land
test and the zeta position gate were therefore wrong, breaking the vertical
free-/partial-slip boundary correction for 3D C-grid velocities.

Fixed to match the analogous, correctly-formed eta/xsi corrections in the same
function:

    is_land(0, 0, 1, 0) & is_land(0, 0, 1, 1) & (zeta > 0)

Existing interpolation and convert tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@VeckoTheGecko

Copy link
Copy Markdown
Contributor

Hi @steps-re , thanks for the PR! I think @erikvansebille will be able to provide a review on this.

In future could you please make sure to follow our PR template? That makes things easier on our end :)

@erikvansebille erikvansebille left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yep, this change is good; thanks for catching this, @steps-re!

@erikvansebille erikvansebille enabled auto-merge (squash) July 13, 2026 11:44
@erikvansebille erikvansebille merged commit d29d36b into Parcels-code:main Jul 13, 2026
15 of 16 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Parcels development Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants