Skip to content
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.

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

Shift cyclostrophic argument to model_kwargs in from_tracks #936

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ Code freeze date: YYYY-MM-DD

### Deprecated

- `climada.hazard.trop_cyclone.trop_cyclone_windfields.compute_angular_windspeeds.cyclostrophic` argument
- `climada.entity.exposures.Exposures.meta` attribute
- `climada.entity.exposures.Exposures.set_lat_lon` method
- `climada.entity.exposures.Exposures.set_geometry_points` method
Expand Down
4 changes: 4 additions & 0 deletions climada/hazard/trop_cyclone/trop_cyclone.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,10 @@ def from_tracks(
inside of the brackets is that the wind speed maximum is attained a bit
farther away from the center than according to the recorded radius of
maximum winds (RMW). Default: False
cyclostrophic : bool, optional
If True, do not apply the influence of the Coriolis force (set the Coriolis
terms to 0).
Default: True for H10 model, False otherwise.

ignore_distance_to_coast : boolean, optional
If True, centroids far from coast are not ignored.
Expand Down
26 changes: 17 additions & 9 deletions climada/hazard/trop_cyclone/trop_cyclone_windfields.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"""

import logging
import warnings
from typing import Optional, Tuple, Union

import numpy as np
Expand Down Expand Up @@ -99,8 +100,8 @@ def compute_angular_windspeeds(
d_centr: np.ndarray,
mask_centr_close: np.ndarray,
model: int,
cyclostrophic: Optional[bool] = False,
model_kwargs: Optional[dict] = None,
cyclostrophic: bool = False,
Copy link
Collaborator

@emanuel-schmid emanuel-schmid Dec 13, 2024

Choose a reason for hiding this comment

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

The proper way to deal with this is probably to mark the argument as deprecated rather than removing it right away. So we can keep it compatible.
If it is provided despite its deprecation insert it in model_kwargs, e.g., like this:

if cyclostrophic is not None:
    warnings.warn("yada yada", DeprecationWarning)
    model_kwargs["cyclostrophic"] = cyclostrophic

):
"""Compute (absolute) angular wind speeds according to a parametric wind profile

Expand All @@ -117,16 +118,25 @@ def compute_angular_windspeeds(
Wind profile model selection according to MODEL_VANG.
model_kwargs: dict, optional
If given, forward these kwargs to the selected model. Default: None
cyclostrophic : bool, optional
If True, do not apply the influence of the Coriolis force (set the Coriolis terms to 0).
Default: False
cyclostrophic: bool, optional, deprecated
This argument is deprecated and will be removed in a future release.
Include `cyclostrophic` as `model_kwargs` instead.

Returns
-------
ndarray of shape (npositions, ncentroids)
containing the magnitude of the angular windspeed per track position per centroid location
"""
model_kwargs = {} if model_kwargs is None else model_kwargs

if cyclostrophic is not None:
warnings.warn(
"The 'cyclostrophic' argument is deprecated and will be removed in a future"
"release. Include it in 'model_kwargs' instead.",
DeprecationWarning,
)
model_kwargs["cyclostrophic"] = cyclostrophic

Copy link
Collaborator

Choose a reason for hiding this comment

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

The proper way to deal with this is probably to mark the argument as deprecated rather than removing it right away. So we can keep it compatible. If it is provided despite its deprecation insert it in model_kwargs, e.g., like this:

if cyclostrophic is not None:
    warnings.warn("yada yada", DeprecationWarning)
    model_kwargs["cyclostrophic"] = cyclostrophic

Like this @emanuel-schmid ? see also the other edits

compute_funs = {
MODEL_VANG["H1980"]: _compute_angular_windspeeds_h1980,
MODEL_VANG["H08"]: _compute_angular_windspeeds_h08,
Expand All @@ -139,7 +149,6 @@ def compute_angular_windspeeds(
si_track,
d_centr,
mask_centr_close,
cyclostrophic=cyclostrophic,
**model_kwargs,
)
result[0, :] *= 0
Expand Down Expand Up @@ -241,11 +250,11 @@ def _compute_angular_windspeeds_h10(
si_track: xr.Dataset,
d_centr: np.ndarray,
close_centr_msk: np.ndarray,
cyclostrophic: bool = True,
gradient_to_surface_winds: float = DEF_GRADIENT_TO_SURFACE_WINDS,
rho_air_const: float = DEF_RHO_AIR,
vmax_from_cen: bool = True,
vmax_in_brackets: bool = False,
**kwargs,
):
"""Compute (absolute) angular wind speeds according to the Holland et al. 2010 model

Expand Down Expand Up @@ -285,8 +294,8 @@ def _compute_angular_windspeeds_h10(
ndarray of shape (npositions, ncentroids)
containing the magnitude of the angular windspeed per track position per centroid location
"""
if not cyclostrophic:
LOGGER.debug(
if not kwargs.get("cyclostrophic", True):
LOGGER.warning(
"The function _compute_angular_windspeeds_h10 was called with parameter "
'"cyclostrophic" equal to false. Please be aware that this setting is ignored as the'
" Holland et al. 2010 model is always cyclostrophic."
Expand Down Expand Up @@ -1203,7 +1212,6 @@ def _compute_windfields(
mask_centr_close,
model,
model_kwargs=model_kwargs,
cyclostrophic=False,
)

# Influence of translational speed decreases with distance from eye.
Expand Down
Loading