-
Notifications
You must be signed in to change notification settings - Fork 9
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
Forecast Precisions #3759
Forecast Precisions #3759
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3759 +/- ##
=======================================
Coverage 79.66% 79.66%
=======================================
Files 297 297
Lines 10823 10823
Branches 525 525
=======================================
Hits 8622 8622
Misses 2056 2056
Partials 145 145 ☔ View full report in Codecov by Sentry. |
Quality Gate passedIssues Measures |
@@ -187,9 +187,9 @@ export const marshalMoreCast2ForecastRecords = (forecasts: MoreCast2ForecastRow[ | |||
for_date: forecast.forDate.toMillis(), | |||
precip: forecast.precip.value, | |||
rh: forecast.rh.value, | |||
temp: forecast.temp.value, | |||
temp: Math.round(forecast.temp.value), |
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'm not sure we need to be rounding values here. I think I would error on the side of maintaining precision in the event that a forecaster is using persistence forecasting or values from one of the weather models.
Or is the reason to do the rounding to prevent decimals from being sent to and appearing in the WFWX forecasting app?
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.
Yes, to prevent decimals from appearing in WFWX, and following my interpretation of the acceptance criteria "Forecaster can only forecast integers for temp and wind speed". Without rounding, the front end would show integers but they would technically be forecasting floats if they're using models.
I also figured the discrepancy between Morecast and WFWX might be puzzling.
Open to changing it of course, that was just my interpretation of the desired functionality
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.
Having WF1 and Morecast match is good enough reason for me! Thanks for the clarification!
Limits precision for temp and wind speed (both as integers)
Closes #3697
Test Links:
Landing Page
MoreCast
Percentile Calculator
C-Haines
FireBat
FireBat bookmark
Auto Spatial Advisory (ASA)
HFI Calculator