-
Notifications
You must be signed in to change notification settings - Fork 143
Add JOSS paper draft to calibration module PR #876
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 JOSS paper draft to calibration module PR #876
Conversation
| def calibrate(hazard: Hazard, exposure: Exposure, impact_data: pd.DataFrame): | ||
| """Calibrate an impact function with BayesianOptimizer""" | ||
|
|
||
| def impact_function_tropical_cyclone(v_half: float, scale: float) -> ImpactFuncSet: |
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.
| def impact_function_tropical_cyclone(v_half: float, scale: float) -> ImpactFuncSet: | |
| def impact_function_tropical_cyclone(v_half: float, scale: float) -> ImpactFuncSet: |
For the example, I would rather use the generic ImpactFunc.from_sigmoid_impf.
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.
The code replicates the main part of the tutorial, and the plots are almost exactly the same. We can change it, but then it actually becomes harder to replicate the example. However, I could also change the tutorial to use from_sigmoid_impf.
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 would do that. In part because the impact function for TC is quite particular, and in part because of the near-future plans to move TC to the petals repo.
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 can then actually showcase a calibration with v_thresh, v_half and constraints!
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.
@chahank Actually, the sigmoid implementation in from_sigmoid_impf is really not useful. It has no threshold parameter, meaning that you can easily create a function where mdr(0) != 0. It would be more useful to generalize the Emanuel-style sigmoid definition, as we did in the other paper. But this will also require the definition of a new function. So, for the sake of simplicity, I would keep everything as is.
Co-authored-by: Chahan M. Kropf <chahan.kropf@usys.ethz.ch>
Changes proposed in this PR:
This PR fixes #
PR Author Checklist
develop)PR Reviewer Checklist