Skip to content

bugfix: Raise on inconsistent metadata #2958

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

Open
VeckoTheGecko wants to merge 2 commits into
base: main
Choose a base branch
from
Open

bugfix: Raise on inconsistent metadata #2958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
85dbf02
BUG: Raise on inconsistent metadata
VeckoTheGecko Oct 9, 2026
865457c
Add comment
VeckoTheGecko Oct 9, 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
3 changes: 3 additions & 0 deletions src/parcels/_core/model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
)
from parcels._logger import logger
from parcels._python import NOTSET, NotSetType
from parcels._sgrid.accessor import assert_metadata_ds_consistency

@erikvansebille erikvansebille Oct 9, 2026

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.

So this function already existed, but was not run? Good that we then caught it now

@VeckoTheGecko VeckoTheGecko Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It existed because it was being used in the accessor for the isel calls that the accessor provided.

The oversight was since the accessor was developed separately to from_sgrid_conventions and the convert module

from parcels.interpolators import (
CGrid_Velocity,
Ux_Velocity,
Expand Down Expand Up @@ -265,6 +266,8 @@ def from_sgrid_conventions(
# ds["lon"] = ds[node_dimensions[0]]
# ds["lat"] = ds[node_dimensions[1]]

assert_metadata_ds_consistency(ds, ds.sgrid.metadata)

vector_fields = resolve_vector_fields(ds, vector_fields)
assert_valid_vector_fields(ds, vector_fields)

Expand Down
16 changes: 8 additions & 8 deletions src/parcels/_datasets/structured/generic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,11 +312,11 @@ def _unrolled_cone_curvilinear_grid():
topology_dimension=2,
node_dimensions=("XG", "YG"),
face_dimensions=(
sgrid.FaceNodePadding("XC", "XG", sgrid.Padding.BOTH),
sgrid.FaceNodePadding("YC", "YG", sgrid.Padding.BOTH),
sgrid.FaceNodePadding("XC", "XG", sgrid.Padding.NONE),
sgrid.FaceNodePadding("YC", "YG", sgrid.Padding.NONE),
),
node_coordinates=("lon", "lat"),
vertical_dimensions=(sgrid.FaceNodePadding("ZC", "ZG", sgrid.Padding.BOTH),),
vertical_dimensions=(sgrid.FaceNodePadding("ZC", "ZG", sgrid.Padding.NONE),),
),
),
"ds_2d_outer": xr.Dataset(
Expand Down Expand Up @@ -367,11 +367,11 @@ def _unrolled_cone_curvilinear_grid():
topology_dimension=2,
node_dimensions=("XG", "YG"),
face_dimensions=(
sgrid.FaceNodePadding("XC", "XG", sgrid.Padding.NONE),
sgrid.FaceNodePadding("YC", "YG", sgrid.Padding.NONE),
sgrid.FaceNodePadding("XC", "XG", sgrid.Padding.BOTH),
sgrid.FaceNodePadding("YC", "YG", sgrid.Padding.BOTH),
),
node_coordinates=("lon", "lat"),
vertical_dimensions=(sgrid.FaceNodePadding("ZC", "ZG", sgrid.Padding.NONE),),
vertical_dimensions=(sgrid.FaceNodePadding("ZC", "ZG", sgrid.Padding.BOTH),),
),
),
"2d_left_unrolled_cone": _unrolled_cone_curvilinear_grid().pipe(
Expand Down Expand Up @@ -402,8 +402,8 @@ def _unrolled_cone_curvilinear_grid():
_DATASET_NAME_TO_SGRID_NAME = {
"ds_2d_left": "ds_2d_padded_high",
"ds_2d_right": "ds_2d_padded_low",
"ds_2d_outer": "ds_2d_padded_none",
"ds_2d_inner": "ds_2d_padded_both",
"ds_2d_outer": "ds_2d_padded_both",
"ds_2d_inner": "ds_2d_padded_none",
}


Expand Down
6 changes: 3 additions & 3 deletions src/parcels/convert.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,10 +513,10 @@ def croco_to_sgrid(*, fields: dict[str, xr.Dataset | xr.DataArray], coords: xr.D
node_dimensions=("lon", "lat"),
node_coordinates=("lon", "lat"),
face_dimensions=(
sgrid.FaceNodePadding("xi_u", "xi_rho", sgrid.Padding.HIGH),
sgrid.FaceNodePadding("eta_v", "eta_rho", sgrid.Padding.HIGH),
sgrid.FaceNodePadding("xi_u", "xi_rho", sgrid.Padding.NONE),
sgrid.FaceNodePadding("eta_v", "eta_rho", sgrid.Padding.NONE),
),
vertical_dimensions=(sgrid.FaceNodePadding("s_rho", "depth", sgrid.Padding.HIGH),),
vertical_dimensions=(sgrid.FaceNodePadding("s_rho", "depth", sgrid.Padding.NONE),),
).to_attrs(),
)

Expand Down
3 changes: 3 additions & 0 deletions tests/test_advection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ def test_advection_zonal_periodic():
halo.lon.values = ds.lon.values[1] + 1
halo.XG.values = ds.XG.values[1] + 2
ds = xr.concat([ds, halo], dim="XG", data_vars="all")
ds = ds.drop_vars("XC").assign_coords(
XC=("XC", ds.XG.values + 0.5, {"axis": "X"})
) # update cell centers to adjust for added halo

fieldset = FieldSet.from_sgrid_conventions(ds, mesh="flat")

Expand Down
11 changes: 11 additions & 0 deletions tests/test_fieldset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from parcels._datasets.structured.generic import datasets as datasets_structured
from parcels._datasets.structured.generic import datasets_sgrid
from parcels._datasets.unstructured.generic import datasets as datasets_unstructured
from parcels._sgrid.accessor import SGridDatasetInconsistency

ds = datasets_structured["ds_2d_left"]

Expand Down Expand Up @@ -343,6 +344,16 @@ def test_fieldset_from_sgrid_conventions(ds_name):
assert len(fieldset.fields) > 0


@pytest.mark.parametrize("face_dim", ["face_dimension1", "face_dimension2", "vertical_dimensions_dim2"])
def test_fieldset_from_sgrid_conventions_inconsistent_padding(face_dim):
# HIGH padding requires face and node dimensions to be the same size
ds = datasets_sgrid["ds_2d_padded_high"]
ds = ds.isel({face_dim: slice(1, None)}) # plain xarray isel, so face dim is one shorter than the node dim

with pytest.raises(SGridDatasetInconsistency, match=f"face dimension {face_dim!r}"):
FieldSet.from_sgrid_conventions(ds, mesh="flat")


def test_fieldset_skip_field_data_validation():
ds = datasets_structured["ds_2d_left"]
ds["U_A_grid"][:] = np.nan
Expand Down
Loading