From 686c35693c2d5ce32b3b679431f0640192b09d74 Mon Sep 17 00:00:00 2001 From: Yulian Lyubomirov Cenov Date: Fri, 22 May 2026 12:32:54 +0200 Subject: [PATCH 1/4] Add units.py to validate and handle conversions only between compatible unit types Signed-off-by: Yulian Lyubomirov Cenov --- src/physrisk/risk_models/generic_risk_model.py | 3 ++- src/physrisk/utils/units.py | 16 ++++++++++++++++ .../config_based_vuln_model_acute.py | 3 ++- 3 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 src/physrisk/utils/units.py diff --git a/src/physrisk/risk_models/generic_risk_model.py b/src/physrisk/risk_models/generic_risk_model.py index 36cbf28d..5aeb9af4 100644 --- a/src/physrisk/risk_models/generic_risk_model.py +++ b/src/physrisk/risk_models/generic_risk_model.py @@ -34,6 +34,7 @@ ) from physrisk.kernel.risk import Measure, MeasureKey, RiskMeasureCalculator from pint import UnitRegistry +from physrisk.utils.units import needs_conversion ureg = UnitRegistry() @@ -445,7 +446,7 @@ def calc_measure( param = float( np.interp(return_period, resp.return_periods, resp.intensities) ) - if resp.units != "default": + if needs_conversion(resp.units, bounds.units): param = ureg.convert(param, resp.units, bounds.units) if math.isnan(param): return Measure( diff --git a/src/physrisk/utils/units.py b/src/physrisk/utils/units.py new file mode 100644 index 00000000..8eab291a --- /dev/null +++ b/src/physrisk/utils/units.py @@ -0,0 +1,16 @@ +ADIM_UNITS = {"index"} + + +def needs_conversion(source_units: str, target_units: str | None) -> bool: + if source_units == "default" or target_units in (None, "default"): + return False + + if source_units == target_units: + return False + + if source_units in ADIM_UNITS or target_units in ADIM_UNITS: + raise ValueError( + f"Cannot convert between incompatible units: {source_units} and {target_units}" + ) + + return True diff --git a/src/physrisk/vulnerability_models/config_based_vuln_model_acute.py b/src/physrisk/vulnerability_models/config_based_vuln_model_acute.py index b1f4dfc6..2faab572 100644 --- a/src/physrisk/vulnerability_models/config_based_vuln_model_acute.py +++ b/src/physrisk/vulnerability_models/config_based_vuln_model_acute.py @@ -39,6 +39,7 @@ from physrisk.vulnerability_models.impact_function_selector import ( ImpactFunctionSelector, ) +from physrisk.utils.units import needs_conversion ureg = UnitRegistry() logger = logging.getLogger(__name__) @@ -152,7 +153,7 @@ def get_distributions( "Future hazard curve is not monotonic; adjusting to ensure non-decreasing curve." ) - conversion = curve.indicator_units is not None and future.units != "default" + conversion = needs_conversion(future.units, curve.indicator_units) if conversion: fut_intensities = ureg.convert( fut_intensities, future.units, curve.indicator_units From 827cde9384a465295754c30cae62f384aa2310ca Mon Sep 17 00:00:00 2001 From: Juan Roman Date: Mon, 25 May 2026 10:43:12 +0200 Subject: [PATCH 2/4] add docstring Signed-off-by: Juan Roman --- src/physrisk/utils/units.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/physrisk/utils/units.py b/src/physrisk/utils/units.py index 8eab291a..bea4cd9a 100644 --- a/src/physrisk/utils/units.py +++ b/src/physrisk/utils/units.py @@ -2,6 +2,23 @@ def needs_conversion(source_units: str, target_units: str | None) -> bool: + """Return whether values should be converted between two unit labels. + + Units marked as ``default`` and missing target units do not require + conversion. Equal units also do not require conversion. Dimensionless index + units cannot be converted to or from dimensional units. + + Args: + source_units: Units attached to the source values. + target_units: Units expected by the target consumer, or ``None`` if + unspecified. + + Returns: + ``True`` when unit conversion should be attempted, otherwise ``False``. + + Raises: + ValueError: If source and target are provided, source is not ``default``, and only one uses adimensional units. + """ if source_units == "default" or target_units in (None, "default"): return False From 7c4f650b8b75752b4caa4462f6b5bdd4e2583e17 Mon Sep 17 00:00:00 2001 From: Juan Roman Date: Mon, 25 May 2026 10:46:33 +0200 Subject: [PATCH 3/4] add reproducing test Signed-off-by: Juan Roman --- .../test_config_based_vulnerability.py | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/vulnerability_models/test_config_based_vulnerability.py b/tests/vulnerability_models/test_config_based_vulnerability.py index 34e68a5e..cf57caaa 100644 --- a/tests/vulnerability_models/test_config_based_vulnerability.py +++ b/tests/vulnerability_models/test_config_based_vulnerability.py @@ -34,6 +34,7 @@ RiverineInundation, Wind, ) +from physrisk.kernel.hazard_model import HazardEventDataResponse from physrisk.kernel.impact_distrib import ImpactType from physrisk.vulnerability_models.config_based_impact_curves import ( PiecewiseLinearImpactCurve, @@ -812,6 +813,42 @@ def test_create_vulnerability_model(): ) +def test_config_based_vulnerability_accepts_index_units(): + config_items = [ + VulnerabilityConfigItem( + hazard_class="Fire", + asset_class="Asset", + asset_identifier="type=Generic,location=Generic", + indicator_id="susceptibility", + indicator_units="index", + impact_id="damage", + impact_units=None, + curve_type="indicator/piecewise_linear", + points_x=[0.0, 0.5, 1.0], + points_y=[0.0, 0.2, 1.0], + ) + ] + factory = VulnerabilityModelsFactory(config=config_items) + vulnerability_models = factory.vulnerability_models() + model = vulnerability_models.vuln_model_for_asset_of_type(Asset)[0] + asset = Asset(location="Generic", latitude=0.0, longitude=0.0) + hazard_response = HazardEventDataResponse( + return_periods=np.array([10.0, 100.0, 1000.0]), + intensities=np.array([0.2, 0.6, 0.9]), + units="index", + ) + + vulnerability, event = model.get_distributions(asset, [hazard_response]) + + np.testing.assert_array_equal( + vulnerability.intensity_bins, np.array([0.2, 0.6, 0.9, 0.9]) + ) + np.testing.assert_array_almost_equal( + vulnerability.impact_bins, np.array([0.08, 0.36, 0.84, 0.84]) + ) + assert event.units == "index" + + @pytest.mark.skip("example, not test") def test_read_write_utilities(tmp_path): config = basic_vulnerability_config() From 408ca63231e7be0b58572dedd0c60fb46fce81ff Mon Sep 17 00:00:00 2001 From: Juan Roman Date: Thu, 28 May 2026 09:17:03 +0200 Subject: [PATCH 4/4] change naming Signed-off-by: Juan Roman --- src/physrisk/utils/units.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/physrisk/utils/units.py b/src/physrisk/utils/units.py index bea4cd9a..32046d1f 100644 --- a/src/physrisk/utils/units.py +++ b/src/physrisk/utils/units.py @@ -1,12 +1,12 @@ -ADIM_UNITS = {"index"} +UNITLESS = {"index", ""} def needs_conversion(source_units: str, target_units: str | None) -> bool: """Return whether values should be converted between two unit labels. Units marked as ``default`` and missing target units do not require - conversion. Equal units also do not require conversion. Dimensionless index - units cannot be converted to or from dimensional units. + conversion. Equal units also do not require conversion. Unitless indicators (e.g. index) + cannot be converted to or from dimensional units, and attempting to do so raises an error. Args: source_units: Units attached to the source values. @@ -17,7 +17,7 @@ def needs_conversion(source_units: str, target_units: str | None) -> bool: ``True`` when unit conversion should be attempted, otherwise ``False``. Raises: - ValueError: If source and target are provided, source is not ``default``, and only one uses adimensional units. + ValueError: If source and target are provided, source is not ``default``, and only one is unitless. """ if source_units == "default" or target_units in (None, "default"): return False @@ -25,9 +25,9 @@ def needs_conversion(source_units: str, target_units: str | None) -> bool: if source_units == target_units: return False - if source_units in ADIM_UNITS or target_units in ADIM_UNITS: + if source_units in UNITLESS or target_units in UNITLESS: raise ValueError( - f"Cannot convert between incompatible units: {source_units} and {target_units}" + f"Conversion is not defined between incompatible units: {source_units} and {target_units}" ) return True