Skip to content

Conversation

@erikvansebille
Copy link
Member

This PR adds an explicit XLinear_Velocity interpolator, effectively moving the code from inside field.py. This improves transparency what is being done in interpolation.

if self._vector_interp_method is None:
u = self.U._interp_method(particle_positions, grid_positions, self.U)
v = self.V._interp_method(particle_positions, grid_positions, self.V)
if "3D" in self.vector_type:
w = self.W._interp_method(particle_positions, grid_positions, self.W)
else:
w = 0.0
else:
(u, v, w) = self._vector_interp_method(particle_positions, grid_positions, self)

First of all, do you agree this would be a good change?

A few more more things to do

  • base the choice between Grid_Velcoity and XLinear_Velocity would be based on grid metadata, in FieldSet creation. @VeckoTheGecko, could you help?
  • Make similar functions for UxGrids. @fluidnumerics-joe, could you pick that up?
  • Remove the code linked above from field.py (so that VectorFields can't have _vector_interp_method=None

Following the discussion around zonal and meridional spherical conversion in #2455
erikvansebille added a commit that referenced this pull request Jan 13, 2026
Temporarily, as #2461 will move this unit conversion to the vector-interpolators
@fluidnumerics-joe
Copy link
Contributor

fluidnumerics-joe commented Jan 13, 2026

Making sure I understand.. This PR would remove the fallback to component-wise interpolation and require a vector field interpolator to be defined ?

@erikvansebille erikvansebille marked this pull request as ready for review January 13, 2026 13:57
@erikvansebille
Copy link
Member Author

Making sure I understand.. This PR would remove the fallback to component-wise interpolation and require a vector field interpolator to be defined ?

No, it would make it more explicit. I moved the code we already had into its own Ux_Velocity interpolator:

def Ux_Velocity(
particle_positions: dict[str, float | np.ndarray],
grid_positions: dict[_UXGRID_AXES, dict[str, int | float | np.ndarray]],
vectorfield: VectorField,
):
"""Interpolation kernel for Vectorfields of velocity on a UxGrid."""
u = vectorfield.U._interp_method(particle_positions, grid_positions, vectorfield.U)
v = vectorfield.V._interp_method(particle_positions, grid_positions, vectorfield.V)
if vectorfield.grid._mesh == "spherical":
u /= 1852 * 60 * np.cos(np.deg2rad(particle_positions["lat"]))
v /= 1852 * 60
if "3D" in vectorfield.vector_type:
w = vectorfield.W._interp_method(particle_positions, grid_positions, vectorfield.W)
else:
w = 0.0
return u, v, w

The advantage is that users can now more easily see how velocities are interpolated (since Interplators are much more public-facing), and that they can also more easily make adaptations (such as e.g. changing from spherical conversion to ellipsoid conversion in lines 723/724)

OK?

Copy link
Contributor

@VeckoTheGecko VeckoTheGecko left a comment

Choose a reason for hiding this comment

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

Just did a full review - looks good! One question out of my own curiousity

@erikvansebille erikvansebille merged commit ab2cba3 into v4-dev Jan 13, 2026
11 checks passed
@erikvansebille erikvansebille deleted the add_XLinearVelocity branch January 13, 2026 14:18
@github-project-automation github-project-automation bot moved this from Backlog to Done in Parcels development Jan 13, 2026
vector_interp_method: Callable | None = None,
):
_assert_str_and_python_varname(name)
if vector_interp_method is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't this require that users provide a vector interpolation method here and not allow fall back onto component wise interpolation ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, but the vector_interp_method can fall back on the component wise interpolation. That's exactly what's done in

def Ux_Velocity(
particle_positions: dict[str, float | np.ndarray],
grid_positions: dict[_UXGRID_AXES, dict[str, int | float | np.ndarray]],
vectorfield: VectorField,
):
"""Interpolation kernel for Vectorfields of velocity on a UxGrid."""
u = vectorfield.U._interp_method(particle_positions, grid_positions, vectorfield.U)
v = vectorfield.V._interp_method(particle_positions, grid_positions, vectorfield.V)
if vectorfield.grid._mesh == "spherical":
u /= 1852 * 60 * np.cos(np.deg2rad(particle_positions["lat"]))
v /= 1852 * 60
if "3D" in vectorfield.vector_type:
w = vectorfield.W._interp_method(particle_positions, grid_positions, vectorfield.W)
else:
w = 0.0
return u, v, w

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.

4 participants