Skip to content

Conversation

@cfkanesan
Copy link

Description

High level functions implementing vertical interpolation to pressure, potential temperature and arbitrary fields.

Contributor Declaration

By opening this pull request, I affirm the following:

  • All authors agree to the Contributor License Agreement.
  • The code follows the project's coding standards.
  • I have performed self-review and added comments where needed.
  • I have added or updated tests to verify that my changes are effective and functional.
  • I have run all existing tests and confirmed they pass.

@cfkanesan cfkanesan changed the title Add vertical interpolation (initial commit) Add vertical interpolation Nov 25, 2025
@sandorkertesz sandorkertesz self-requested a review November 26, 2025 11:05
@sandorkertesz sandorkertesz linked an issue Dec 2, 2025 that may be closed by this pull request
@sandorkertesz
Copy link
Collaborator

sandorkertesz commented Dec 3, 2025

Hi @cfkanesan, thank you very much for this contribution. These would be the first methods implemented in the high-level earthkit-meteo API!

Please note there is an ongoing development to add somewhat similar methods to the array level interface: #81. So it would be important to agree on a naming convention for the names and args/kwargs of all these methods.

These are just my initial thoughts for interpolate_k2p.

interpolate_k2p

def interpolate_k2p(
    field: xr.DataArray,
    mode: Literal["linear_in_p", "linear_in_lnp", "nearest_sfc"],
    p_field: xr.DataArray,
    p_tc_values: Sequence[float],
    p_tc_units: Literal["Pa", "hPa"],
) -> xr.DataArray:

earhkit-meteo tends to use long method names and short args and kwargs names. So in the same spirit the name could be interpolate_from_k_to_pressure_levels. However, I am not sure if k is descriptive enough (at least I am not familiar with it).

Maybe k could be e.g. model? But unfortunately "model" is ambiguous. There are already some methods in meteo.vertical doing computations with ECMWF model levels, e.g. pressure_at_model_levels. I think those methods should be renamed having e.g. eta instead of model in the name. In that case k could be kept.

  • field: maybe this should be data to be more generic?
  • p_field: maybe p_data`?
  • mode: maybe this should be interpolation? As for the options, I suggest linear, 'logandnearest` for brevity, assuming these names are descriptive enough.
  • p_tc_values: I suggest p_target
  • p_tc_units: I suggest p_target_units

Also consider assigning a default value to mode (linear). Maybe also to p_tc_units?

@sandorkertesz
Copy link
Collaborator

Hi @cfkanesan , as the outcome of some recent discussions we suggest the following naming convention and arg/kwarg ordering for the methods in the PR:

  • the input vertical levels should be called sleve (since as we understand ICON uses SLEVE coords)
  • the kwargs of a method should start with the input data/coords followed by the target related parameters
  • we suggest "theta" for potential temperature. However, there are methods in "earthkit.meteo.vertical", which use potential temperature in the name and th as a kwarg. E.g.: def temperature_from_potential_temperature(th, p). We will need to change here th to the more descriptive theta. As for the method name, we are not sure what to do, but we would like to use same naming conventions in vertical and meteo. This requires a separate discussion.

The proposal

# current name: interpolate_monotonic
def interpolate_monotonic(
    data,
    coord,
    target_coord,
    interpolation)
# current name: interpolate_to_pressure_levels
def interpolate_to_pressure_levels(
  data,
  p,
  target_p,
  target_p_units.
  interpolation
)
# current name: interpolate_to_any
def interpolate_sleve_to_coord_levels(
    data,
    h,
    target_data,
    target_coord,
    folding_mode
) 
# current name: interpolate_to_theta_levels
def interpolate_sleve_to_theta_levels(
    data,
    theta,
    h,
    target_theta,
    target_theta_units,
    folding_mode,
)

@cfkanesan
Copy link
Author

Hi @sandorkertesz, thanks for the feedback. Perhaps coord could be renamed to target_data to have consistency between interpolate_sleve_to_coord_levels and interpolate_monotonic? Otherwise, I agree with the suggestions.

@sandorkertesz
Copy link
Collaborator

sandorkertesz commented Jan 6, 2026

Hi @sandorkertesz, thanks for the feedback. Perhaps coord could be renamed to target_data to have consistency between interpolate_sleve_to_coord_levels and interpolate_monotonic? Otherwise, I agree with the suggestions.

Hi @cfkanesan , I think using target_data instead of coord in interpolate_monotonic would be misleading since coord describes the source.

@cfkanesan
Copy link
Author

From my understanding, both coord and target_data represent the values of the target coordinate field on the same levels as the input data. Perhaps the ordering of the arguments is confusing, because in interpolate_sleve_to_theta_levels, the position of this argument is swapped with h.

@sandorkertesz
Copy link
Collaborator

Hi @cfkanesan, having checked the code, using target_data in my original proposal in interpolate_sleve_to_coord_levels is wrong. I would simply use the term coord in both cases:

def interpolate_monotonic(
    data,
    coord,
    target_coord,
    interpolation)

def interpolate_sleve_to_coord_levels(
    data,
    h,
    coord,
    target_coord,
    folding_mode
) 

... since coord describes the coordinates on the same levels as the source data ( and it is always the same type of coordinate as target_coord).

values=tc_values.tolist(),
)

return interpolate_sleve_to_coord_levels(data, theta, tc, h, folding_mode)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should not it be:

return interpolate_sleve_to_coord_levels(data, h, theta, tc, folding_mode)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants