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

Add functions to compute, plot, store the local hazard exceedence intensity and RP maps #857

Closed
wants to merge 14 commits into from

Conversation

simonameiler
Copy link
Collaborator

@simonameiler simonameiler commented Mar 7, 2024

Changes proposed in this PR:

  • addition of new functions to store the hazard maps that are generated in the Hazard.plot_rp_intensity() function as netCDF write_netcdf_local_exceedance_inten or raster files write_raster_local_exceedance_inten.
  • addition of new functions to compute local return period maps local_return_period and _loc_return_period for given hazard intensities, incl. plotting function plot_local_rp.
  • addition of new functions to store these local return period maps for given hazard intensities as NetCDF write_netcdf_local_return_periods or raster files write_raster_local_return_periods.

This PR fixes #854

PR Author Checklist

PR Reviewer Checklist

@simonameiler simonameiler changed the title Add function to store the local exceedence intensity maps as netcdf file Add function to store the local exceedence intensity and RP maps Mar 21, 2024
@simonameiler simonameiler changed the title Add function to store the local exceedence intensity and RP maps Add functions to compute, plot, store the local hazard exceedence intensity and RP maps Mar 21, 2024
@luseverin luseverin marked this pull request as ready for review April 2, 2024 14:55
@luseverin luseverin marked this pull request as draft April 2, 2024 14:58
@@ -1541,6 +1543,48 @@ def local_exceedance_inten(self, return_periods=(25, 50, 100, 250)):
Reason: no negative intensity values were found in hazard.')
inten_stats[inten_stats < 0] = 0
return inten_stats

def local_return_period(self, hazard_intensities):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a confusing method signature to me. Why would a method of the class Hazard require the user to supply hazard intensities? These are already an attribute of the object.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here the variable hazard_intensities does not refer to the intensities of the Hazard object but to the threshold intensities for which the return period per grid point should be calculated. To avoid confusion we could rename this to "threshold_intensities"?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 threshold_intensities would be much better. 😄

Comment on lines +1552 to +1553
hazard_intensities : np.array
Hazard intensities to consider.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be careful, this might lead to very large memory usage. Hazard intensities are on purpose sparse matrices.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment above, hazard_intensities usually only includes a few values.

@emanuel-schmid
Copy link
Collaborator

@simonameiler looks like the branch is not yet up to date with the develop branch and, e.g., changes from have been reversed.
I'd suggest to merge from develop the sooner the better. Merging conflicts usually get worse the longer the merging is postponed. 😟

@simonameiler
Copy link
Collaborator Author

@simonameiler looks like the branch is not yet up to date with the develop branch and, e.g., changes from have been reversed. I'd suggest to merge from develop the sooner the better. Merging conflicts usually get worse the longer the merging is postponed. 😟

Thank you for the hint. I handed the pull request off to @ValentinGebhart

@ValentinGebhart
Copy link
Collaborator

ValentinGebhart commented Jun 20, 2024

I don't know how to resolve the issue that, even after merging develop back into this branch, many of the changes made to develop are not transferred here. One solution would be to create a new branch from develop (say "feature/write_haz_rp_maps2") and add the new functions again there. Do you think that this is fine or do you see a better solution? @emanuel-schmid @chahank

@chahank
Copy link
Member

chahank commented Jun 20, 2024

@ValentinGebhart : keeping it simple is good to me, so just making a new branch is fine for me. Just then delete the old one.

@ValentinGebhart
Copy link
Collaborator

PR completed in a different branch (PR #898). I saved a local copy of the functions here, if needed please contact me.

@ValentinGebhart ValentinGebhart deleted the feature/write_haz_rp_maps branch July 16, 2024 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants