Skip to content

bugfix: Raise on inconsistent metadata - #2958

Open
VeckoTheGecko wants to merge 2 commits into
Parcels-code:mainfrom
VeckoTheGecko:padding-mismatch
Open

VeckoTheGecko wants to merge 2 commits into
Parcels-code:mainfrom
VeckoTheGecko:padding-mismatch

Conversation

@VeckoTheGecko

Copy link
Copy Markdown
Contributor

Description

Raise SGridDatasetInconsistency error for instances where data doesn't match attached metadata.

This also flagged other tests in the codebase which had a problem with metadata.

Checklist

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): Fixing tests

@VeckoTheGecko VeckoTheGecko changed the title BUG: Raise on inconsistent metadata bugfix: Raise on inconsistent metadata Oct 9, 2026
)
from parcels._logger import logger
from parcels._python import NOTSET, NotSetType
from parcels._sgrid.accessor import assert_metadata_ds_consistency

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

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

@VeckoTheGecko

Copy link
Copy Markdown
Contributor Author

Hmmm. So the failure in the docs is quite interesting

something for me to look into next week

pixi run docs-clean failure

Symptom

The docs build stopped with make: *** [html] Error 2. Sphinx reported:

myst_nb.core.execute.base.ExecutionError: docs/user_guide/examples/tutorial_stuck_particles.ipynb

The notebook cell that failed is the one that builds the NEMO C-grid fieldset:

ds_fset = parcels.convert.nemo_to_sgrid(
    fields={"U": cufields["uo"], "V": cvfields["vo"]},
    coords=ds_coords,
)
fieldset = parcels.FieldSet.from_sgrid_conventions(ds_fset)

The underlying error:

SGridDatasetInconsistency: Node dimension 'depth' has size 1, and face dimension 'depth_center'
has size of 75. Due to dataset padding of <Padding.HIGH: 'high'>, expected face dimension
depth_center to actually be size 1.

It is raised from FieldSet.from_sgrid_conventions → StructuredModelData.from_sgrid_conventions
(src/parcels/_core/model.py) → assert_metadata_ds_consistency (src/parcels/_sgrid/accessor.py).

The zmq.error.ZMQError: Socket operation on non-socket traceback in the output is only noise
from the kernel being shut down after the failure.

Root cause

The new consistency check on this branch (commit 85dbf0205, "BUG: Raise on inconsistent
metadata") found inconsistent metadata that parcels.convert.nemo_to_sgrid had always produced
for this dataset:

  1. In the NemoNorthSeaORCA025-N006_data tutorial dataset, uo has dims
    (time_counter, depthu, y, x) with 75 depth levels. The dataset has no depthw.
  2. _maybe_bring_other_depths_to_depth (src/parcels/convert.py) renames
    depthu/depthv to depth_center, which has size 75.
  3. There is no depthw to become the depth (node) dimension. The same function therefore
    falls back to adding a placeholder depth=[0] of size 1, with the log message
    "No depth dimension found in your dataset. Assuming no depth (i.e., surface data)".
  4. The converter then attaches the SGRID metadata
    FaceNodePadding("depth_center", "depth", Padding.HIGH). With HIGH padding, the number of
    faces should equal the number of nodes, so 75 faces against 1 node is inconsistent.

So the check did its job. The converter's fallback only handled the case where there is no
vertical dimension at all, not the case where there are cell-centre depths but no interfaces.

Fix

In _maybe_bring_other_depths_to_depth, when depth_center exists but depth does not, the
converter now builds a depth node dimension of the same size. The first interface is at 0 and
each later one sits halfway between consecutive cell centres. It logs an INFO message
recommending that users pass depthw in coords for the exact interface depths. The single-level
surface placeholder is now only used when there is no vertical dimension at all.

The test test_nemo_to_sgrid_depth_centers_without_depthw in tests/test_convert.py covers this.

Trade-off: the midpoint interfaces are only an estimate, because real NEMO W-levels are not
exactly halfway between the centres. The alternative is to raise an error asking for depthw
whenever there are centre depths but no depthw.

Other notebooks

Sphinx executes notebooks in parallel and stops at the first error, so some notebooks hadn't
finished when the build stopped. After the fix, the full pixi run docs-clean build succeeded,
so none of the other notebooks hit the new check.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

BUG: FieldSet constructed without error despite inconsistent padding

2 participants