Skip to content

Conversation

@albreverman
Copy link

Interpret additional variables from HRRR grids. Use "wind" in variable name to correctly interpret wind speed grids.

Resolves: HMS-4620

Interpret additional variables from HRRR grids. Use "wind" in variable
name to correctly interpret wind speed grids.

Resolves: HMS-4620
Copy link
Collaborator

@tombrauer tombrauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@albreverman I left a few review comments. Let me know if you have any questions.

GEFS("ge.*\\.pgrb2.*\\.0p.*\\.f.*\\..*", ".*"),
LIVNEH_PRECIP("prec.\\d{4}.nc", ".*"),
HRRR_WRFSFCF("hrrr.*wrfsfcf.*", ".*"),
HRRR_APCP("hrrr.*", ".*APCP.*"), // Precipitation
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every regex added to this list dramatically increases processing times for large datasets. Because of the performance implications, one of my goals is to stop adding variables to this list and to use this approach instead. If you really do think HRRR should be added to this list (which I would understand given it's relevance) can you try to consolidate to just one regex instead of many? That will be less impactful on performance.

return DssDataType.PER_CUM;
if (variable == TEMPERATURE)
return DssDataType.INST_VAL;
if (variable == HUMIDITY)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From this table in the user's manual it looks like humidity, radiation, pressure, temperature, and windspeed can be instantaneous or period data. I would try re-writing this method to accommodate those cases. You'll see some of the other variables like SWE have a second conditional for interval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants