Combining several interpolation functions using a new util function #918

ValentinGebhart · 2024-07-16T13:22:01Z

Changes proposed in this PR:

Adaption of the functions Impact.local_exceedance_impact, Hazard.local_exceedance_intensity, Hazard.local_return_period to use the new util module climada.util.interpolate and the three functions therein interpolate_ev, stepfunction_ev and group_frequency.
This makes the functions local_exceedance_imp, loc_return_imp, _cen_return_imp, local_exceedance_inten, _loc_return_inten, _cen_return_inten and _loc_return_period obsolete.
A deprecation warning for the old versions Impact.local_exceedance_imp, Hazard.local_exceedance_inten is added.
All methods Impact.local_exceedance_impact, Hazard.local_exceedance_intensity, Hazard.local_return_period now output a geodataframe, such that one can use the util function plot_from_gdf for plotting. This makes the plotting functions Hazard.plot_rp_intensity and Impact.plot_rp_imp obsolete. A deprecation warning was added.
In the methods Impact.local_exceedance_imp, Hazard.local_exceedance_inten, Hazard.local_return_period, before interpolation, we group together values with the same intensity/impact (by summing their frequencies). This was not done in the functions before

Open questions/further possibilities:

Do we want to try a more efficient output type than gdf if the above function output gdfs with a lot of zeros (e.g. spare gdf etc)?
Could one do the fitting matrix-wise (or block-wise) instead of column-wise for better efficiency? Not clear due to requirements of underlying fit functions.
Future Option: PR to add a fit method (to some extreme value distribution, specified by the user).
Future Option: One could use the util method also in Impact.calc_freq_curve to give it a bit more flexibility. Not clear if needed.
Future Option: We could now easily add a method Impact.local_return_period, similar to the Hazard case. Not sure if needed.

This PR fixes #904, #915 and partially also #209

PR Author Checklist

PR Reviewer Checklist

chahank · 2024-07-17T13:30:38Z

climada/engine/impact.py

+            )
+
+        # create the output GeoDataFrame
+        gdf = gpd.GeoDataFrame(geometry = [Point(x, y) for y, x in self.coord_exp], crs = self.crs)


I think the following is more efficient and clean:

https://geopandas.org/en/stable/docs/reference/api/geopandas.points_from_xy.html

chahank · 2024-07-17T13:31:26Z

climada/engine/impact.py

@@ -813,7 +847,8 @@ def plot_basemap_impact_exposure(self, event_id=1, mask=None, ignore_zero=False,
                                                 buffer, extend, zoom, url, axis=axis, **kwargs)

        return axis
-
+
+    # TODO: replace with subplots_from_gdf()


The name subplots_from_gdf is a bit confusing to me. Why would this be a subplot instead of a plot?

The subplots_from_gdf already exists, it is how we now plot local return periods for given hazard intensity thresholds. I named it subplots because it produces a plot with several subplots, each for a different column of the gdf. I am happy to change the name though, just "plot_from_gdf" or "plots_from_gdf"?

I would then prefer plot_from_gdf. Plot is quite clear, and can contain more than one. Plurals are in my opinion to be avoided as much as possible as they easily create confusion.

# Conflicts: # CHANGELOG.md

in order to avoid circular imports

…imada_python into develop-white

…ance

sarah-hlsn

I would add "impacts" to the examples given for values in the docstrings; since there are just two options for values I think we might as well mention both. Other than that everything looks good to me!

climada/util/interpolation.py

Co-authored-by: Sarah Hülsen <[email protected]>

emanuel-schmid · 2024-11-04T14:26:50Z

climada/hazard/plot.py

@@ -32,6 +36,7 @@ class HazardPlot:
    Contains all plotting methods of the Hazard class
    """

+    # TODO depreciating warning, to be replaced with plot_from_gdf


But strictly speaking this isn't a TODO anymore?

emanuel-schmid · 2024-11-04T14:35:25Z

climada/hazard/plot.py

+        """
+        This function is deprecated,
+        use Impact.local_exceedance_impact and util.plot.plot_from_gdf instead.
+        """
+        LOGGER.warning(
+            "The use of Hazard.plot_rp_intensity is deprecated."
+            "Use Hazard.local_exceedance_intensity and util.plot.plot_from_gdf instead."
+        )


I'd suggest to use the @deprecated decorator instead, like e.g., in the climada.hazard.centr module.
I know, atm, the climada codebase is heterogeneous in this regard but in the long run the decorator is meant to take over.

thank you, I didn't know about this. I changed this to decorators now

emanuel-schmid · 2024-11-04T15:01:34Z

are the changes in the plots of the Hazard tutorial relevant? If they aren't I'd suggest to undo them.
(@ValentinGebhart I already did, ready to push, just let me know.)

emanuel-schmid · 2024-11-04T16:02:10Z

climada/util/checker.py

+def convert_frequency_unit_to_time_unit(frequency_unit):
+    """Converts common frequency units to corresponding time units. Unknown frequency
+    units are converted to "years".


wouldn't the climada.util.dates_times package be a better match for this method?

I agree, I will move the method there (and the unit test accordingly), thank you!

emanuel-schmid · 2024-11-05T08:11:51Z

Great Job! Many thanks!

ValentinGebhart added 10 commits July 5, 2024 15:37

first draft general fit method

fe3becc

updated fit func and new loc_exceedance func

eab38ac

added local_rp fct and stepfunction fit

316ca45

update local_exceedance_imp func

7f9649d

Merge branch 'develop' into feature/restructure_fitfuncs_exceedance

4167295

change fcts to return gdfs

482a207

prepare to merge with develop

05b2800

Merge branch 'develop' into feature/restructure_fitfuncs_exceedance

a3efe90

merged develop into branch

9ba4ddb

update plot local impact exceedance

74b4b81

ValentinGebhart changed the title ~~Combining several fit functions into a new util fit functions~~ Combining several fit functions into a new util fit function Jul 16, 2024

made function calls consistent with new structure

acf3664

chahank reviewed Jul 17, 2024

View reviewed changes

plot NaNs in gray in geo_im_from_array

1e0d238

ValentinGebhart marked this pull request as draft July 18, 2024 09:34

ValentinGebhart added 13 commits July 18, 2024 14:28

added method options for exceed funcs

332b0ab

fixed bug when y_scale is log

f0bf633

added some tests for fit function

b09a7c6

group values with same inten/impact

0c6377c

updated tests

0fca30b

added util function to group frequencies

2790fdd

changed default settings, added old functions

5c29173

updated tests

23f7f5d

added deprecation warnings and changed sorting

199f8e2

new asymptotic and small-input behaviour of funcs

adfc6e0

added docstrings

0e6ed2c

more thorough tests of exceedance and rp funcs

97d1c68

Merge branch 'develop' into feature/restructure_fitfuncs_exceedance

6ede419

ValentinGebhart marked this pull request as ready for review July 29, 2024 15:10

emanuel-schmid and others added 16 commits October 4, 2024 11:15

Merge branch 'develop' into feature/restructure_fitfuncs_exceedance

efb9921

# Conflicts: # CHANGELOG.md

format

822c7f1

import Impact from climada.engine.impact

1b78bdf

in order to avoid circular imports

avoid circular imports

87cb6fd

Merge branch 'develop' into develop-black

b9be992

pre-commit run --all-files

5143b99

setup: DEPS_TEST update

bc60010

avoid circular imports

3c5fc8f

Merge branch 'develop-white' of https://github.com/CLIMADA-project/cl…

aa0615f

…imada_python into develop-white

format exceptions

dcbe2c5

added tutorial about new functionalities

13dfc8e

Merge branch 'develop-white' into feature/restructure_fitfuncs_exceed…

d9d600d

…ance

blacked my feature branch

81695f4

Merge branch 'develop-black' into feature/restructure_fitfuncs_exceed…

e0ccfa0

…ance

Merge branch 'develop' into feature/restructure_fitfuncs_exceedance

f76723c

removed tutorial for future PR

928f24e

sarah-hlsn approved these changes Nov 4, 2024

View reviewed changes

climada/util/interpolation.py Outdated Show resolved Hide resolved

climada/util/interpolation.py Outdated Show resolved Hide resolved

climada/util/interpolation.py Outdated Show resolved Hide resolved

climada/util/interpolation.py Outdated Show resolved Hide resolved

Apply suggestions from code review

685b6c8

Co-authored-by: Sarah Hülsen <[email protected]>

emanuel-schmid reviewed Nov 4, 2024

View reviewed changes

ValentinGebhart and others added 3 commits November 4, 2024 16:20

changed deprecation warning to decorators

bfef378

changelog: amend deprecated section

386730b

update changelog with three more deprecated entries

4b88cb0

emanuel-schmid reviewed Nov 4, 2024

View reviewed changes

moved time frequency conversion to util dates_times

4d29cbf

ValentinGebhart merged commit d195226 into develop Nov 5, 2024
18 checks passed

ValentinGebhart deleted the feature/restructure_fitfuncs_exceedance branch November 5, 2024 08:14

ValentinGebhart mentioned this pull request Nov 5, 2024

Add local_return_period method to Impact objects and new tutorial #971

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Combining several interpolation functions using a new util function #918

Combining several interpolation functions using a new util function #918

Uh oh!

ValentinGebhart commented Jul 16, 2024 •

edited

Loading

Uh oh!

chahank Jul 17, 2024

Uh oh!

chahank Jul 17, 2024

Uh oh!

ValentinGebhart Jul 17, 2024

Uh oh!

chahank Jul 17, 2024

Uh oh!

sarah-hlsn left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

emanuel-schmid Nov 4, 2024

Uh oh!

emanuel-schmid Nov 4, 2024

Uh oh!

ValentinGebhart Nov 4, 2024

Uh oh!

emanuel-schmid commented Nov 4, 2024 •

edited

Loading

Uh oh!

emanuel-schmid Nov 4, 2024

Uh oh!

ValentinGebhart Nov 4, 2024

Uh oh!

emanuel-schmid commented Nov 5, 2024

Uh oh!

Uh oh!

Uh oh!

Combining several interpolation functions using a new util function #918

Combining several interpolation functions using a new util function #918

Uh oh!

Conversation

ValentinGebhart commented Jul 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Author Checklist

PR Reviewer Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarah-hlsn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emanuel-schmid commented Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emanuel-schmid commented Nov 5, 2024

Uh oh!

Uh oh!

Uh oh!

ValentinGebhart commented Jul 16, 2024 •

edited

Loading

emanuel-schmid commented Nov 4, 2024 •

edited

Loading