Repository navigation
add kwarg to local exceedance intensity to change value ordering - #1285
ValentinGebhart wants to merge 2 commits into
Conversation
luseverin
left a comment
There was a problem hiding this comment.
I've had a look and it looks sensible to me. The examples added in the notebook are helpful. I would also add in the Notes section of the docstrings of the functions when you are supposed to set reverse=True, e.g.
Use reverse=True when smaller hazard intensities correspond to higher impacts, e.g. in the case of negative winter temperatures or drought indices such as SPEI.
or something similar. Otherwise it looks good to me.
|
|
emanuel-schmid
left a comment
There was a problem hiding this comment.
many thanks! and sorry for the delayed review.
| "binning. Please choose a larger value of bin_decimals=%s.", | ||
| bin_decimals, | ||
| ) | ||
| return frequency, value |
There was a problem hiding this comment.
looks dangerous to me. shouldn't there be a reverse case handling?
while we're at it - wouldn't it even be preferable to downright raise an error here instead of writing warnings to a log that might never be read?
| raise ValueError("No test values or test frequencies are given.") | ||
|
|
||
| # sort values and frequencies | ||
| sorted_idxs = np.argsort(values) |
There was a problem hiding this comment.
what would happen if the complete reverse case handling was omitted below and sorted_idxs were reversed instead?



Changes proposed in this PR:
reverseto methodsHazard.local_exceedance_intensity,Hazard.local_return_period,util.interpolation.preprocess_and_interpolate_evsuch that the user can decide how to order intensities, for instance when handling negative intensities like the SPIE dorught index.climada_util_local_exceedance_values.ipynbThis PR addresses #1257.
PR Author Checklist
develop)PR Reviewer Checklist