-
Notifications
You must be signed in to change notification settings - Fork 63
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
Lumped Forcings Engine Data Provider #845
Lumped Forcings Engine Data Provider #845
Conversation
… error messages Co-authored-by: Phil Miller - NOAA <[email protected]>
5dfb5c4
to
b025920
Compare
using TestFixture = ForcingsEngineLumpedDataProviderTest; | ||
|
||
constexpr const char* TestFixture::config_file; | ||
const std::time_t TestFixture::time_start = data_access::detail::parse_time("2024-01-17 01:00:00"); |
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.
One note that I just realized I need to change: time_start
has no actual impact on the Forcings Engine's time domain, so while this is currently incorrect, the instance still starts at 2023-01-17 01:00:00. From what I can think of currently, there isn't an easy way to assert that the time matches, since only the time duration is exposed from the forcings engine (i.e. get_data_start_time
and others report in seconds with an epoch of 0).
This relates back to #858.
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.
@jduckerOWP For the Forcings Engine in particular, could it expose a start_time
variable via BMI that we can use to cross-check that the model engine's simulation start time matches the forcings engine's configured/available-data start time?
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.
Alternately, could it expose that as a pseudo-variable that we can set via BMI, rather than living in the config file?
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.
@PhilMiller we certainly can expose a start time via BMI for cross-checking timestamps between the Forcings Engine and the framework. Regarding @program-- comment for the timestamp discrepancy we can discuss this more in our meeting today but generally the NWM operational forecast iterations are initialize at some start time (0z,6z,12z,18z etc.) and the first available forecast data is at forecast hour 1 of the simulation. Therefore, generally hour 0 (or start time of simulation) will basically need to be forced with some "hot start" fields if models require forcings at the actual start time. For retrospective datasets however, we can modify the Forcings Engine to directly provide meteorological forcings at that start time hour we'll just need some revisions to the workflow to make that work.
Other than the |
"T2D_ELEMENT", | ||
"Q2D_ELEMENT", | ||
"PSFC_ELEMENT", | ||
"RAINRATE_ELEMENT" |
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.
With the recent upgrades pushed to the NextGen Forcings Engine, some operational configurations (HRRR, RAP, GFS, NBM) can now also extract and advertise a new BMI field called "LQFRAC_ELEMENT" (and "LQFRAC_NODE" if unstructured mesh domain configuration). This field is described as the "liquid fraction of precipitation" and has units of % (0-100 range). For this first iteration of the NextGen Forcings Engine, it'll be okay if we leave it out for this PR and we can just merge the data later on if desired. The complexity of that variable field is that at times, it will or won't be available pending on the NWM operational configuration we apply. Will be happy to discuss these details further in our meeting!
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.
Since it won't always be present, we shouldn't require it in the test.
…t_value[s] to use beginning time as epoch
…le path and scratch at configure and build time
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 requested one small change but it is not important. Otherwise I see no issues
: ÷_id[separator + 1] | ||
); | ||
|
||
return std::atol(split); |
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 normally prefer strtol() to atol(). For most linux compiliers the actual conversion will be a call internal call to strtol so it is easier for strtol calls to be implemented.
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 this is a good point. I'll make an issue as a reminder to change this. Thanks 😄
This PR adds the
ForcingsEngineLumpedDataProvider
class, inheriting fromForcingsEngineDataProvider<double, CatchmentAggrDataSelector>
.This provider interacts with the python-based Forcings Engine when running in "hydrofabric" mode (i.e. in the configuration file,
GRID_TYPE="hydrofabric"
). The following variables are accessible through the data provider interface:U2D_ELEMENT
U2D
V2D_ELEMENT
V2D
T2D_ELEMENT
T2D
Q2D_ELEMENT
Q2D
LWDOWN_ELEMENT
LWDOWN
SWDOWN_ELEMENT
SWDOWN
PSFC_ELEMENT
PSFC
RAINRATE_ELEMENT
RAINRATE
Within the engine's catchment domain, each variable has an aggregated scalar value for each catchment. As such, every constructed lumped data provider is associated with a single catchment (but, the underlying forcings engine instance is shared between them).
This PR also includes a minor change to the parent base class, which moves the time and config file path member variables into theprotected
scope to allow printing of them in derived functions/exceptions and prevent the need to perform some conversions withinstd::chrono
if they were kept in theprivate
scope.Additions
data_access::ForcingsEngineLumpedDataProvider
ForcingsEngineLumpedDataProviderTest
test fixture using AORC data from AWS S3.Testing
Notes
In order to run the forcings engine unit tests prerequisites must be met:
$WGRIB2
environment variable with the path to yourwgrib2
executable, e.g.Checklist