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

Applying a measure throws an error if impact function id is not of type int #925

Open
mncosta opened this issue Jul 24, 2024 · 2 comments
Open
Labels
accepting pull request Contribute by raising a pull request to resolve this issue! bug documentation

Comments

@mncosta
Copy link

mncosta commented Jul 24, 2024

Describe the bug
When applying a measure that aims to map from one impact function to another, error is thrown if impact functions id are not of type int. Yet from the docs, id could be of type int | str.

To Reproduce
Steps to reproduce the behavior/error:

  1. Have a measure with imp_fun_map set to change a impact function with a str id, i.e., "HZtoHZ1"

Why doesn't the param imp_fun_map receive a tuple(int | str, int |str), just like the other params:

  • hazard_inten_imp : tuple(float, float)
  • mdd_impact : tuple(float, float)
  • paa_impact : tuple(float, float)

This would enable having either int or str as types of impact functions id, and one would bypass the need to parse the string imp_fun_map as well?

Code example (following the first example on the docs)

%matplotlib inline
import numpy as np
from climada.entity import ImpactFuncSet, ImpfTropCyclone, Exposures
from climada.entity.measures import Measure
from climada.hazard import Hazard

# define measure
meas = Measure(
    name='Mangrove',
    haz_type='TC',
    color_rgb=np.array([1, 1, 1]),
    cost=500000000,
    imp_fun_map='HZtoHZ1',
)

# impact functions
impf_tc = ImpfTropCyclone.from_emanuel_usa()
impf_tc.id = 'HZ'
impf_all = ImpactFuncSet([impf_tc])
impf_all.plot();

# dummy Hazard and Exposures
haz = Hazard('TC') # this measure does not change hazard
exp = Exposures() # this measure does not change exposures

# new impact functions
new_exp, new_impfs, new_haz = meas.apply(exp, impf_all, haz)
axes = new_impfs.plot();
axes.set_title('TC: Modified impact function')

Expected behavior
Impact function id on the new exposure objecto should be mapped to new id even if ids are strings.

Screenshots
image

Climada Version: 4.1.0

System Information (please complete the following information):

  • Operating system and version: Ubuntu
  • Python version: 3.9

Additional context
None

@mncosta mncosta added the bug label Jul 24, 2024
@chahank
Copy link
Member

chahank commented Jul 24, 2024

Thanks for reporting this. The simplest is to always use integers for impact function ids. We might update this in the future and require integers for the impact function ids.

We are also in the process of revising the measure class and will for sure fix this in the process.

@peanutfun
Copy link
Member

In many cases, using a string as ID might work, but we generally assume that the IDs are integers. Therefore, I suggest we continue not enforcing a type, but update the docs of ImpactFunc such that id is expected to be int.

@peanutfun peanutfun added accepting pull request Contribute by raising a pull request to resolve this issue! documentation labels Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepting pull request Contribute by raising a pull request to resolve this issue! bug documentation
Projects
None yet
Development

No branches or pull requests

3 participants