Default C-grid tracer fields to CGrid_Tracer interpolator by v3ravani · Pull Request #3053 · Parcels-code/Parcels

v3ravani · 2026-11-10T09:37:19Z

Description

Problem

In StructuredModelData.from_sgrid_conventions, all scalar fields defaulted to XLinear, even for tracer fields located at cell centers on staggered C-grids. Following Delandmeter and Van Sebille (2019), C-grid tracers should default to CGrid_Tracer to be interpolated constant over the grid cell.

Solution

Added _select_xinterpolator in src/parcels/_core/model.py to identify tracer fields defined at cell centers (face dimensions) on staggered C-grids and default them to CGrid_Tracer.
A-grid datasets (where velocity components are collocated on identical dimensions) default all scalar fields to XLinear.
Node-located fields, edge-located fields, vertical interface fields, and unstaggered grids continue to default to XLinear.
In peninsula_dataset, aligned streamfunction P dimensions to ["YG", "XG"] to match its underlying node coordinates lat (YG) and lon (XG) (consistent with stommel_gyre_dataset).

Testing

Added regression tests in tests/test_fieldset.py covering default interpolators on C-grids (test_cgrid_tracer_default_interpolator), dimensional edge cases (test_select_xinterpolator_edge_cases), and A-grid behavior (test_agrid_all_fields_default_to_xlinear).
Verified test suite passes locally (tests/test_fieldset.py, tests/test_advection.py, tests/test_interpolation.py, tests/test_field.py, tests/test_xgrid.py).
Verified with pixi run lint and pixi run typing.

Checklist

Closes CGrid Tracers do not default to CGrid_Tracer Interpolator #3049
Tests added
This PR targets the correct branch (main for normal development, v3-support for v3 support)

AI Disclosure

This PR contains AI-generated content.
- I have tested any AI-generated content in my PR.
- I take responsibility for any AI-generated content in my PR.
- Describe how you used it (e.g., by pasting your prompt): I used an AI coding assistant to help draft _select_xinterpolator, align dataset coordinates, and author regression tests. The code was developed at my direction, and I have read, tested, and validated all changes locally.

When constructing fields from SGRID conventions in StructuredModelData, default tracer fields defined at cell centers (face dimensions) on staggered C-grids to CGrid_Tracer rather than hardcoding all scalar fields to XLinear.

erikvansebille · 2026-11-10T11:26:06Z

Thanks for this PR< but you have not filled out the AI statement in our PR template. As per our AI-usage policy, that is very important for us

v3ravani · 2026-11-10T16:10:17Z

Thanks for this PR< but you have not filled out the AI statement in our PR template. As per our AI-usage policy, that is very important for us

Thanks, I have updated the PR description with the AI disclosure.

…terpolator

…taset P coordinates

v3ravani · 2026-11-10T17:18:46Z

Updated with root-cause fix and synced with latest main:

Handled A-grid datasets in _select_xinterpolator so collocated scalar fields default to XLinear.
In peninsula_dataset, aligned streamfunction P dimensions to ['YG', 'XG'] to match its underlying node coordinates lat and lon (consistent with stommel_gyre_dataset), ensuring continuous linear interpolation along particle streamlines.
Verified all unit tests (test_peninsula_fieldset, test_fieldset, test_interpolation, test_field, test_xgrid), typing (mypy), and linting (pre-commit) pass cleanly.

wyatt-fluidnumerics

Hello, thanks for this PR! See comments.

Broadly I think some of the testing introduced in this PR is out of scope since it focuses more on the correctness of the CGrid_Tracer interpolator rather than the interpolator selection.

It also broke CI but that seemed to be resolved as I finalized this review.

wyatt-fluidnumerics · 2026-11-10T17:06:05Z

+                    model.grid.sgrid_metadata,
+                    model.vector_field_components,
+                )
+                if interp_cls is not None:


_select_xinterpolator never returns None

Fixed, directly instantiated without the redundant None check.

wyatt-fluidnumerics · 2026-11-10T17:09:03Z

+        return XLinear
+
+    # Velocity components (U, V, W or components of vector fields) are not tracers
+    velocity_names = {"U", "V", "W"}


I don't think this is needed. CGrid horizontal velocities will be caught in the second check above since they are not on the face, and the vertical velocity gets caught below.

Agreed, removed the velocity names check.

wyatt-fluidnumerics · 2026-11-10T17:10:05Z

+@pytest.mark.parametrize(
+    "dataset_name",
+    [
+        "ds_2d_left",


These datasets all store their velocities/tracers on CGrids, so they all check the exact same logic, only one should be used.

Updated to test only \ds_2d_left.

wyatt-fluidnumerics · 2026-11-10T17:11:50Z

+    assert isinstance(fieldset.UV.interp_method, CGrid_Velocity)
+
+
+def test_cgrid_tracer_piecewise_constant_evaluation():


I think this is out of scope for this PR.

Removed as out of scope.

wyatt-fluidnumerics · 2026-11-10T17:12:52Z

+    )
+
+    # Unstaggered grid metadata (face == node) -> XLinear
+    from parcels._sgrid.core import FaceNodePadding, Padding, SGrid2DMetadata


This import should be moved to the top of the file.

Moved to the top of the file.

wyatt-fluidnumerics · 2026-11-10T17:13:16Z

+    assert _select_xinterpolator(da_unstaggered, unstaggered_metadata) is XLinear
+
+
+def test_cgrid_tracer_particle_sampling():


Out of scope.

Removed as out of scope.

wyatt-fluidnumerics · 2026-11-10T17:18:41Z

+    over the grid cell using CGrid_Tracer. Non-tracer fields, velocity components,
+    node-located variables, or fields on unstaggered grids default to XLinear.
+    """
+    fnp_x, fnp_y = sgrid_metadata.face_dimensions


I am confused on what this is checking for, my understanding is that if a single dimension is assigned to both the faces and nodes it would break assertions during Fieldset creation, so I believe this is unneeded.

Good catch, removed.

v3ravani · 2026-11-10T17:51:09Z

Thanks for the review and guidance @wyatt-fluidnumerics!

I have addressed all the feedback in commit 996ee7b2:

Cleaned up _select_xinterpolator:
- Removed the redundant if interp_cls is not None: check in model.py.
- Removed the redundant face == node check and hardcoded velocity name sets; selection now relies cleanly on dimension placement and SGRID face/node coordinates.
Scoped tests strictly to interpolator selection:
- De-parametrized test_cgrid_tracer_default_interpolator to use only ds_2d_left.
- Removed the out-of-scope piecewise constant evaluation and particle sampling tests.
- Moved all SGRID imports to the module top in tests/test_fieldset.py.
- Retained targeted tests for interpolator selection edge cases and A-grid defaults.
Verification:
- All tests in tests/test_fieldset.py, tests/test_advection.py, tests/test_interpolation.py, tests/test_field.py, and tests/test_xgrid.py pass.
- Code formatting, linting (pre-commit), and typing (mypy) pass with zero issues.

Default C-grid tracer fields to CGrid_Tracer interpolator

df59982

When constructing fields from SGRID conventions in StructuredModelData, default tracer fields defined at cell centers (face dimensions) on staggered C-grids to CGrid_Tracer rather than hardcoding all scalar fields to XLinear.

github-project-automation Bot added this to Parcels development Nov 10, 2026

github-project-automation Bot moved this to Backlog in Parcels development Nov 10, 2026

erikvansebille requested a review from wyatt-fluidnumerics November 10, 2026 16:13

v3ravani added 2 commits November 10, 2026 22:41

Merge remote-tracking branch 'upstream/main' into fix-cgrid-tracer-in…

9109734

…terpolator

Fix CI failures: handle A-grid scalar defaults and align peninsula da…

15fe1d1

…taset P coordinates

wyatt-fluidnumerics requested changes Nov 10, 2026

View reviewed changes

github-project-automation Bot moved this from Backlog to Ready in Parcels development Nov 10, 2026

Address review comments: simplify interpolator selection and scope tests

996ee7b

v3ravani requested a review from wyatt-fluidnumerics November 11, 2026 13:33

		assert isinstance(fieldset.UV.interp_method, CGrid_Velocity)


		def test_cgrid_tracer_piecewise_constant_evaluation():

		assert _select_xinterpolator(da_unstaggered, unstaggered_metadata) is XLinear


		def test_cgrid_tracer_particle_sampling():

Conversation

v3ravani commented Nov 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem

Solution

Testing

Checklist

AI Disclosure

Uh oh!

erikvansebille commented Nov 10, 2026

Uh oh!

v3ravani commented Nov 10, 2026

Uh oh!

v3ravani commented Nov 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wyatt-fluidnumerics left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wyatt-fluidnumerics Nov 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

v3ravani commented Nov 10, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

v3ravani commented Nov 10, 2026 •

edited

Loading

v3ravani commented Nov 10, 2026 •

edited

Loading

wyatt-fluidnumerics left a comment •

edited

Loading

wyatt-fluidnumerics Nov 10, 2026 •

edited

Loading