-
Notifications
You must be signed in to change notification settings - Fork 149
TropCyclone: Implement ER11 wind model #577
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
Conversation
tovogt
left a comment
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 added some comments about the meaning of the changes.
| "H10": [27.604317, 28.720708, 29.894993, 27.52234 , 32.512395, 37.114355, | ||
| 23.848917, 29.614752, 33.775593, 32.545347, 19.957627, 41.014578], |
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 values for this test changed because the H10 model had a small bug related to units.
| "H1980": [21.376807, 21.957217, 22.569568, 21.284351, 24.254226, 26.971303, | ||
| 19.220149, 21.984516, 24.196388, 23.449116, 0, 31.550207], |
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 values for this test changed because the H1980 model had a small bug related to units.
| d_centr = np.array([[35, 75, 220], [30, 1000, 300]], dtype=float) | ||
| r_max = np.array([75, 40], dtype=float) |
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.
These variables need to be provided in kilometers. Before this PR, we wrongly assumed that they need to be provided in meters. This was not as bad as it sounds because in most formulas, only the ratio between the two variables is taken into account.
| # convert recorded surface winds to gradient-level winds without translational influence | ||
| t_vmax = track.max_sustained_wind.values.copy() | ||
| # convert surface winds (in m/s) to gradient winds without translational influence | ||
| t_vmax = track.max_sustained_wind.values.copy() * KN_TO_MS |
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.
This was a bug in the H1980 model: We forgot to convert from knots to meters per second.
| return np.clip(hol_b, 1, 2.5) | ||
|
|
||
| def _x_holland_2010(d_centr, r_max, v_max_s, hol_b, close_centr, v_n=17.0, r_n=300e3): | ||
| def _x_holland_2010(d_centr, r_max, v_max_s, hol_b, close_centr, v_n=17.0, r_n=300): |
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.
This was a bug in the H2010 model: We wrongly assumed that all variables are in SI units, but actually, we currently use km for distances everywhere.
| # linearly interpolate between max exponent and peripheral exponent | ||
| x_max = 0.5 | ||
| x[close_centr] = x_max + np.fmax(0, d_centr - r_max) * (x_n - x_max) / (r_n - r_max) |
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.
This doesn't change the behavior, it's just a refactoring.
|
🤞 😁 |
Changes proposed in this PR:
PR Author Checklist
develop)PR Reviewer Checklist