Feature/change impact total value

[chahank]

Changes proposed in this PR:

• Proposal to update the affected value of the exposures to only those centroids with intensity above 0 (or threshold)

This PR would make the impact computation slower by roughly 10%. Do we want that?

The code can be made prettier.

This PR fixes #

PR Author Checklist

• Read the Contribution Guide
• Correct target branch selected (if unsure, select develop)
• Descriptive pull request title added
• Source branch up-to-date with target branch
• Documentation updated
• Tests updated
• Tests passing
• No new linter issues
• Changelog updated

PR Reviewer Checklist

• Read the Contribution Guide
• CLIMADA Reviewer Checklist passed
• Tests passing
• No new linter issues

[chahank]

@peanutfun : an alternative could be to have two methods: one that just computes the total sum of the exposures in the vicinity of a hazard centroid (so as it is currently) and which is used in the impact computation. Another method that would only be for the exposures object and which does what the here proposed method does (compute the value of the exposures affected by a given value of intensity).

[peanutfun]

I like the new thing much better, however, the computation overhead could be a problem. Is Impact.tot_value really something useful? I would suggest keeping the current version of the method for backward-compatiblity, and deprecating Impact.tot_value. Users should instead use the new affected_total_value with a threshold of their choice and retrieve it from the exposure when they actually need it.

[chahank]

I like the new thing much better, however, the computation overhead could be a problem. Is Impact.tot_value really something useful? I would suggest keeping the current version of the method for backward-compatiblity, and deprecating Impact.tot_value. Users should instead use the new affected_total_value with a threshold of their choice and retrieve it from the exposure when they actually need it.

Good idea. Before doing so, it is probably to be discussed once with users from the insurance sector (which are at the origin of this feature). Thanks!

[chahank]

Ok, my suggestion is the following:

We keep the total value as it is currently implemented in the impact computation. We add a new method which does compute the total people subject to a given threshold.

Thus, we do not slow down the impact computation and keep backwards compatibility, and give the possibility to users to obtain the more useful value of affected people directly from the hazard and exposures. Potentially we could also simply ditch completely the tot_value from the impact computation in order to simplify the code base.

[peanutfun]

@chahank I approve!

Potentially we could also simply ditch completely the tot_value from the impact computation in order to simplify the code base.

When? Do you want to create an issue or deprecate the value immediately? With the latter approach, I think we are more likely to get some feedback on that decision 😉

[chahank]

I would go for the more direct approach -> deprecate.

[chahank]

Suggested change

	- `Exposures.affected_total_value` now takes a hazard intensity threshold as argument. [#702](https://github.com/CLIMADA-project/climada_python/pull/702)
	- `Exposures.affected_total_value` now takes a hazard intensity threshold as argument. [#702](https://github.com/CLIMADA-project/climada_python/pull/702)
	- Bugfix for the `Hazard.select` method: any iterable are now accepted for `event_id` and `event_names` [#719](https://github.com/CLIMADA-project/climada_python/pull/719)

[chahank]

I sneaked-in the missing changelog entry from the pull request #719. Hope this is fine for you @peanutfun

[chahank]

@peanutfun ready to merge?

[peanutfun]

I sneaked-in the missing changelog entry from the pull request #719. Hope this is fine for you @peanutfun

I personally much prefer using a separate PR for that, but let's not overcomplicate things. You'll have to add this yourself, I cannot commit your suggestion because it is outdated.

@peanutfun ready to merge?

Yes!