-
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
Hourly FFMC #4153
Hourly FFMC #4153
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4153 +/- ##
==========================================
+ Coverage 80.34% 80.73% +0.39%
==========================================
Files 313 314 +1
Lines 11941 12040 +99
Branches 540 540
==========================================
+ Hits 9594 9721 +127
+ Misses 2159 2131 -28
Partials 188 188 ☔ View full report in Codecov by Sentry. |
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.
Looks great, nice work!
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.
Nice work!
@@ -105,7 +106,7 @@ async def process( | |||
) | |||
|
|||
# Create and store FFMC dataset | |||
ffmc_values, ffmc_no_data_value = calculate_ffmc(ffmc_ds, warped_temp_ds, warped_rh_ds, warped_wind_speed_ds, warped_precip_ds) | |||
ffmc_values, ffmc_no_data_value = calculate_ffmc(ffmc_ds, warped_temp_ds, warped_rh_ds, warped_precip_ds, warped_wind_speed_ds) |
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.
Ouch, good find, I wonder if there's a test we could add to catch this.
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.
We could add some sort of name or id property to WPSDataset
objects and use this to assert that we're getting parameters in the right order?
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.
Yeah I think that'd be worth it since we're passing so many WPSDataset
's around
# 3. Use job start time to determine most recent RDPS model run start time (date and 00z or 12z) | ||
# 4. Use most recent RDPS model run start time to determine most recent hFFMC key to use as source which is always one hour before the RDPS start time (04 or 16 PDT) | ||
# 5. Start calculating hFFMC from model run hour 0 through to 47. Save the calculated hFFMCs to S3. Most recently calculated hFFMC is used as input to the next hour's hFFMC calculation. | ||
# 6. hFFMC rasters are saved to S3 with PDT based keys. |
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.
Are we using PDT for all our other S3 keys? Is there anything preventing/hindering us from using UTC?
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 hourly SFMS rasters we receive are named with PDT times so I followed the existing pattern. I'm totally open to naming via UTC timestamp instead. My brain definitely prefers to work in UTC as it never changes.
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.
Oh, and our other calculated SFMS S3 rasters/keys are daily in nature, so need to consider UTC vs PDT in the naming convention.
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.
Right SFMS gives us PDT rasters. Either way works but I think adding the timezone to the naming convention would be helpful.
Quality Gate passedIssues Measures |
To test locally, set
MAX_MODEL_RUN_HOUR
to 12 or so inrdps_sfms.py
and run/debug the sfms raster processor job option. You might want to deletesfms/calculated/hourlies
from dev S3 storage as well to confirm the new hFFMC rasters are being generated.Fixed:
There is a known bug with warping rH rasters where it is possible to have rH values greater than 100. This breaks hFFMC calculations because the underlying cffdrs ffmc calculation has a range check on rH values to make sure they are between 0 and 100. I'll address this issue in another PR.Calculate and store hourly FFMC rasters.
The general process is:
closes #3517
Test Links:
Landing Page
MoreCast
Percentile Calculator
C-Haines
FireBat
FireBat bookmark
Auto Spatial Advisory (ASA)
HFI Calculator