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

Configuration based TUDelft calculation #353

Merged
merged 10 commits into from
Sep 22, 2024

Conversation

joemoorhouse
Copy link
Collaborator

Finish changes related to running a flood calculation using TUDelft data, including standard of protection, and using configuration-based vulnerability.
Partial implementation of vulnerability function uncertainty in config-based models and add to docs.

Copy link
Collaborator

@xbarra xbarra left a comment

Choose a reason for hiding this comment

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

Please, rebase this branch onto main before merging.

Copy link
Collaborator

@xbarra xbarra left a comment

Choose a reason for hiding this comment

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

I would rather this got moved to it's own pr and merged.

Also a part of a bigger conversation, but squashing it on this merge would make it difficult to track

@@ -0,0 +1,162 @@
from typing import Optional

Choose a reason for hiding this comment

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

Missing docstring in public module (fails Ruff rule D100).

@@ -0,0 +1,162 @@
from typing import Optional
from numba import float64, njit

Choose a reason for hiding this comment

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

numa.float64 imported but unsed (fails Ruff rule F401)

@@ -0,0 +1,162 @@
from typing import Optional
from numba import float64, njit
from numba.experimental import jitclass

Choose a reason for hiding this comment

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

numba.experimental.jitclass imported but unused (fails Ruff rule F401)



class CDFBasedVulnerabilityFunction:
"""In general the vulnerability curve specifies for each hazard intensity level a

Choose a reason for hiding this comment

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

1 blank line required between summary line and description (fails Ruff rule D205)

Use r""" if any backslashes in a docstring (fails Ruff rule D301)

First line should end with a period (fails Ruff rule D400)

First line should end with a period, question mark, or exclamation point (fails Ruff rule D415)

kind: str = "deterministic",
impact_grid: Optional[np.ndarray] = _default_impact_grid,
):
"""Initialise a CDF

Choose a reason for hiding this comment

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

First line should end with a period (fails Ruff rule D400)

First line should end with a period, question mark, or exclamation point (fails Ruff rule D415)

):
self.thresholds = np.array(range(25, 60, 5))

def get_data_requests(

Choose a reason for hiding this comment

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

Missing docstring in public method (fails Ruff rule D102)

indicator_id=self.indicator_id,
)

def get_impact(

Choose a reason for hiding this comment

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

Missing docstring in public method (fails Ruff rule D102)

@@ -68,9 +68,9 @@ def _crs_shape_transform_global(
self,
width: int = 43200,
height: int = 21600,
return_periods: Union[List[float], npt.NDArray] = [0.0],
index_values: Union[List[float], npt.NDArray, List[str]] = [0.0],

Choose a reason for hiding this comment

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

Do not use mutable data structures for argument defaults (fails Ruff rule B006).

@@ -118,10 +118,10 @@ def _crs_shape_transform(
self,
width: int,
height: int,
return_periods: Union[List[float], npt.NDArray] = [0.0],
index_values: Union[List[float], npt.NDArray, List[str]] = [0.0],

Choose a reason for hiding this comment

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

Do not use mutable data structures for argument defaults (fails Ruff rule B006).

" fig.add_scatter(x=vuln.cdf[i, :], y=vuln.impact, name=f\"Flood depth {hazard_indicator[i]}m\")\n",
"# also interpolate for a depth of 0.6m and display\n",
"interp = vuln.interpolate_cdfs(np.array([0.6]))\n",
"fig.add_scatter(x=interp[0, :], y=vuln.impact, name=f\"Interpolated depth 0.6m\")\n",

Choose a reason for hiding this comment

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

f-string without any placeholders (fails Ruff rule F541).

Signed-off-by: Joe Moorhouse <5102656+joemoorhouse@users.noreply.github.com>
…SOP.

Signed-off-by: Joe Moorhouse <5102656+joemoorhouse@users.noreply.github.com>
Signed-off-by: Joe Moorhouse <5102656+joemoorhouse@users.noreply.github.com>
Signed-off-by: Joe Moorhouse <5102656+joemoorhouse@users.noreply.github.com>
Signed-off-by: Joe Moorhouse <5102656+joemoorhouse@users.noreply.github.com>
Signed-off-by: Joe Moorhouse <5102656+joemoorhouse@users.noreply.github.com>
…terable.

Signed-off-by: Joe Moorhouse <5102656+joemoorhouse@users.noreply.github.com>
Signed-off-by: Joe Moorhouse <5102656+joemoorhouse@users.noreply.github.com>
Signed-off-by: Joe Moorhouse <5102656+joemoorhouse@users.noreply.github.com>
Signed-off-by: Joe Moorhouse <5102656+joemoorhouse@users.noreply.github.com>
@joemoorhouse
Copy link
Collaborator Author

Hi @xbarra and @VMarfima,
Thanks for comments; as suggested I pulled out the PDM lock update into a separate PR and rebased. I took the change to add the PDM matrix CI at the same time.

A couple of changes that hit a lot of files:

  1. Change of Type hint in vulnerability models from Iterable[HazardDataResponse] to Sequence[HazardDataResponse]. Probably should have been like this in the first place (not really intent to allow for generators here and length should be known), but had a case with the config-based models where it was desirable to see length of responses hence change in this PR. I also addressed some other Type hints, generally preferring Sequence over List.
    Hopefully does not affect too much and straight forward to change; no good time to make such changes.
  2. Starting to address some of the ruff excluded items I removed exclusion for unused imports.

Added some more doc strings, but still more to do there.

Thanks,
Joe

@joemoorhouse joemoorhouse merged commit e7a1c3e into os-climate:main Sep 22, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants