Bug fix/storm speed #104
Bug fix/storm speed #104
Conversation
…utation uniformly for all unique datetimes even though they can be across different forecasts
…for the first time which was previously set to be 0, now the next time is propagated backward
| if (advisory_data["datetime"] < this_time).sum() == 0: | ||
| shifted_indices[counter] = advisory_data["datetime"][ | ||
| advisory_data["datetime"] > this_time | ||
| ].index[0] |
There was a problem hiding this comment.
@WPringle can you please explain what this does? It says that if there's no time in the dataframe smaller than the current row we're processing, then the shifted time ("previous time") should be the first time that is larger than the current time. Why?! This gives us opposite bearing, right?
There was a problem hiding this comment.
That's a good point about the bearing! This could be a bug that needs fixing.
There was a problem hiding this comment.
It seems all the tests are passing. The coverage test runs into a timeout issue. Do you want me to go ahead with this merge and then later explore the bearing issue in a separate ticket or do you want me to wait for you to test the potential bearing issue?
There was a problem hiding this comment.
Thanks for this! Please wait until I double check everything. I will let you know once happy.
There was a problem hiding this comment.
OK for the bearing when the shifted time is larger than the current time, we can use the forward_azimuth instead of inverse_azimuth. The change has been added in latest commit. Note this is overall a rare case.
| speeds.ffill(inplace=True) | ||
| bearings.ffill(inplace=True) |
There was a problem hiding this comment.
@WPringle where would we get nan to carry forward? We already are getting all indices, we shouldn't run into a case of nan that needs to be filled forward for different isotachs. If possible can you share an example where this happens?
There was a problem hiding this comment.
Does geodetic.inv return nans when locations are same?
There was a problem hiding this comment.
For the same forecast time but different isotachs the result will be nan because the time difference is zero. So we just want to repeat the result from the first line of the same forecast time.
… forward_azimuth for computing bearing (usually inverse_azimuth)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #104 +/- ##
==========================================
- Coverage 91.75% 91.74% -0.01%
==========================================
Files 19 19
Lines 2000 2012 +12
==========================================
+ Hits 1835 1846 +11
- Misses 165 166 +1 ☔ View full report in Codecov by Sentry. |
Fixing storm speed and direction computation for forecasts.
The previous implementation computed only the same speed and direction uniformly for each unique datetime based on the first Lat, Lon values in the unique set, even though these times span across different forecasts with different coordinates.
New implementation computes for each row independently in series with correction where the new forecast starts to find the last previous valid datetime. NaNs that occur for the repeated rows across different isotachs are filled by pandas forward fill function.