Feature/rmax psurgev29 #154
Conversation
…s the other variables and renaming legacy method for RMW to RadiustoMaximumWindsPersistent
…o right column of the data
…inguish between the two RadiusOfMaximumWinds
…ate between the two RadiusOfMaximumWind options
…on changed Vmax, Rmax, and track using the GAHM profile
…e GAHM profile function
…fix to not exceed the maximum isotach
…sotach perturbation method
…ed isotach perturbation upgrade
…otach perturbation upgrade
…ximumSustainedWindSpeed class and including a lower_bound to prevent low and negative values
…ranslation_speed output from the radial Vmax function to be used in Vr calculation
…tside of loop to make it independent of loop order
…ata can be matched, especially relevant for the Rmax over isotach_rad ratio > 1
…d isotach if perturbations are made
…ing nan for the original
…s nan but should be a solution
…n for the perturbed storm in the case it can fit with the GAHM profile
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #154 +/- ##
==========================================
+ Coverage 20.82% 20.96% +0.13%
==========================================
Files 28 28
Lines 3846 3983 +137
==========================================
+ Hits 801 835 +34
- Misses 3045 3148 +103 ☔ View full report in Codecov by Sentry. |
| "cross_track": NaN, | ||
| "along_track": NaN, | ||
| "max_sustained_wind_speed": NaN, | ||
| "radius_of_maximum_winds_persistent": NaN, | ||
| "weight": 1.0 | ||
| } |
There was a problem hiding this comment.
change to distinguish original (NaN) with a perturbed track with a 0.0 perturbation value. These should usually be equivalent but the isotach perturbation method can change isotachs from the original to match with GAHM profile.
| @@ -2,6 +2,6 @@ | |||
| "cross_track": -1.2815515655446004, | |||
| "along_track": 0.5244005127080407, | |||
| "max_sustained_wind_speed": 1.2815515655446004, | |||
| "radius_of_maximum_winds": -0.4, | |||
| "radius_of_maximum_winds_persistent": -0.4, | |||
| "weight": 1.0 | |||
| } | |||
There was a problem hiding this comment.
change to distinguish between the RMW forecast with Gaussian distribution errors and the persistent RMW assumption (same as initial value all the way through to the end of forecast) with Uniform distribution errors
| AlongTrack, | ||
| MaximumSustainedWindSpeed, | ||
| RadiusOfMaximumWindsPersistent, | ||
| ] | ||
|
|
||
| perturbations = perturb_tracks( |
There was a problem hiding this comment.
The existing file in this test uses the persistent RMW assumption. Need to add option to get out a persistent RMW forecast from StormEvents.
| CrossTrack, | ||
| AlongTrack, | ||
| MaximumSustainedWindSpeed, | ||
| RadiusOfMaximumWinds, |
There was a problem hiding this comment.
This online advisory test will get the track with RMW forecast from StormEvents
| @@ -340,15 +365,118 @@ def __init__(self): | |||
| unit=units.knot, | |||
| ) | |||
|
|
|||
| def radial_Vmax_at_BL(self, dataframe: DataFrame) -> Quantity: | |||
There was a problem hiding this comment.
New function in Vmax class to get the radial Vmax after subtracting the translation speed and taking value up to the top of the BL
| B = self.find_parameter_from_GAHM_profile( | ||
| Vr_old, Vmax_old, f_old, Roinv_old, B=B_ini, isotach_rad=isotach_rad, Rrat=Rrat | ||
| ) | ||
| # correct where B is nan |
There was a problem hiding this comment.
It is possible B is NaN because no solution can be found connecting from Vmax at RMW to Vr at r, or if no initial isotach is given. In this case we just replace B with the initial guess. This means we can potentially find solutions for the perturbed isotach (non-zero) even if no initial isotach is given, which can occur e.g. if the perturbation increases Vmax.
| invalid = numpy.isnan(Rrat) | ||
| Rrat[invalid] = Vr_new[invalid] / Vmax_new[invalid] | ||
| Rrat[abs(Rrat - 1.0) < 1e-3] = 0.999 # ensure not exactly 1 | ||
| Rrat[Vr_new > Vmax_new] = numpy.nan # if Vr is stronger than Vmax then ignore |
There was a problem hiding this comment.
The isotach is not compatible with the perturbed Vmax value, which likely occurs if the perturbed Vmax is decreased from the original value.
| Rrat = Rmax_new / isotach_rad | ||
| # correct where Rrat is nan | ||
| invalid = numpy.isnan(Rrat) | ||
| Rrat[invalid] = Vr_new[invalid] / Vmax_new[invalid] |
There was a problem hiding this comment.
If isotach_rad is zero (NaN) then the guess for Rrat will be NaN so this correction allows for a non-zero guess and non-zero solution to be potentially be found for the pertubed isotach radius.
| # enter if it is not the original unperturbed file | ||
| if any(~numpy.isnan(list(perturbation.values()))): | ||
| # Compute potential changes in the central pressure in accordance with Holland B relationship | ||
| dataframe[CentralPressure.name] = self.compute_pc_from_Vmax(dataframe) |
There was a problem hiding this comment.
Moved the central pressure change in accordance with Vmax change outside of variable perturbation loop where it was called after Vmax was perturbated. It is possible the small change in trajectory can affect the radial Vmax and hence the central pressure, so this change will mean the function call is not dependent on order of perturbation (Vmax before or after along_track and cross_track perturbations).
| # Compute potential changes in the central pressure in accordance with Holland B relationship | ||
| dataframe[CentralPressure.name] = self.compute_pc_from_Vmax(dataframe) | ||
|
|
||
| # Compute potential changes to r34/50,64 radii at all quadrants in accordance with the GAHM profile |
There was a problem hiding this comment.
Called outside of variable perturbation loop as changes to any of Vmax, RMW and trajectory can affect how the isotach radii are perturbed.
|
Why was merged again? |
I guess your question is about the email you got about the other similar PR (#139) which I closed. The other merge was your own branch on your fork, I just rebased that one based on the latest ensemble perturbation and then pushed it to the main repo (since I couldn't push to your fork). Then I created this new PR for the branch on main repo to be merged (see #139 (comment)) Do you see any issues? |
|
No, should be good.. |
Rebased #139 after CI fix.