-
Notifications
You must be signed in to change notification settings - Fork 143
Integrates risk trajectories within climada #1037
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
base: develop
Are you sure you want to change the base?
Conversation
Co-authored-by: Lukas Riedel <34276446+peanutfun@users.noreply.github.com>
Co-authored-by: Lukas Riedel <34276446+peanutfun@users.noreply.github.com>
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
|
I created a discussion here #1039 |
- Replaced RiskPeriod class by CalcRiskPeriod class to centralize shared computations and dynamic metric instantiation. - Improved efficiency by reusing shared computations across metrics. - Added InterpolationStrategy and ImpactCalcStrategy classes to abstract that part of the computation
This reverts commit e48c924.
|
Can I ask when you plan to continue and finish the review? |
| # 100% Coverage goal: | ||
| ## Integration test for risk period with periods and freq defined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure to understan the purpose of this file. Are integration tests planed for later on?
| is defined on a per coordinate basis, this one is applied total impact on the whole region | ||
| considered, and proportionally rescales the impacts per coordinate accordingly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| is defined on a per coordinate basis, this one is applied total impact on the whole region | |
| considered, and proportionally rescales the impacts per coordinate accordingly. | |
| is defined on a per coordinate basis, this one is applied total impact on the whole region | |
| considered, and proportionally rescales the impacts per coordinate accordingly. |
What does the rescales the impacts per coordinate accordingly mean ? Attachment and cover would just apply for the whole exposure right? What is being rescaled?
| 4. **Adjustment**: The original impact per exposure point is scaled | ||
| by the ratio of (Residual Risk / Total Impact) or | ||
| (Transferred Risk / Total Impact) for that event. | ||
| This ensures the risk transfer is shared proportionally among all | ||
| impacted exposure points. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is the scaling. Could you please explain what this corresponds too? I am not familiar with this risk transfer spreadind.
Changes proposed in this PR:
Implements
RiskTrajectoryobjects to ease the evaluation of risk / impact in between two points in time.See the associated discussion here: #1039
A notebook showcasing this new module is available in
doc/tutorial/climada_trajectory.ipynb. (It is not a real tutorial yet, but will serve as its basis).Also note that the future option appraisal module will be based on this.
Remaining TODO s
Snapshotby their date when instantiatingExposureshave the same shape within a risk trajectory.group_idare identical forExposuresor handle that case (Handled, different group_id are possible, metric is computed at each date and interpolated in between)PR Author Checklist
develop)PR Reviewer Checklist