NOAA-OWP / cfe

CFE is a conceptual rainfall-runoff model with an implementation of the Basic Model Interface.
Other
21 stars 22 forks source link

BMI Update may incorrectly assume a 1 hour time step #28

Open hellkite500 opened 2 years ago

hellkite500 commented 2 years ago

The Update function assumes a conversion of the precip variable from mm/hr to m/hr by simply accounting for the scale difference between mm and m. However, what happens when update_until modifies the dt for whatever reason to a fractional dt. Is this conversion/assumption still valid? A quick look through the model definitions implies that precipitation should be the value for the timestep -- timestep_rainfall_input_m. Just want to double check that there is nothing untoward happening here.

https://github.com/NOAA-OWP/cfe/blob/3617bda7b887ca283a971cc630fd76d890292db1/src/bmi_cfe.c#L970

jmframe commented 2 years ago

Andy, Keith or Naoki would be better answering this part, but here is my though: The precipitation input unit is kg m-2, which roughly translates to mm, assuming the density is kg m-3. I guess the assumption being that the kg of mass accumulated during the dt. So that note assumes the kg accumulated over a one hour dt. If the dt that you need to run CFE on is less or greater than one hour, than the kg m-2 should reflect the appropriate accumulation. So if you have a 30 min dt, for instance, that kg m-2 should be a 30 minute accumulation, so then dividing by 1000 should give you mm per 30 min, but the trick is that the km m-2 needs to reflect that 30 minute accumulation. Now on the CFE side, this is a question for Fred, but here is my thought: The CFE includes parameters that are calibrated to respond to forcings at one hour timesteps. Since this is a lumped conceptual model, rather than a physical model, there is no great way to change the timesetp. For instance, some of the model parameters are calibrated roughly to simulate water moving down a catchment, but there is no explicit velocity, so the spatial resolution of the catchment and the velocity of the water moving can't be directly compared to give a courant number. So this isn't really as simple as dividing up the precipitation to match the timestep. If you ran CFE at a timestep of one second, for instance, and divided the one hour accumulated precipitation mass by 3600, you probably won't get a comparable surface runoff after that full hour, because the CFE parameters think the precip is actually an hour, so it would give you a result that is more representative of 3600 hours of very low (basically zero) precipitation, rather than one hour of normal precipitation. The point being, it is probably not a good idea to run CFE at a timestep other than 1 hour. But Fred should make this decision.

hellkite500 commented 2 years ago

@jmframe Thanks for your thoughts! That's some sounds reasoning. I hadn't thought through all the implications here, which is why I opened this issue. I just noticed this oddity while looking through code trying to understand the chain of transformations from input data to output. Still some to consider it seems.

SnowHydrology commented 2 years ago

@hellkite500 I agree with @jmframe. CFE has what I would call an implicit time step of one hour. There are parts of the code where the time step is explicitly handled (e.g., Schaake_partitioning_scheme) and others where it is not (e.g., Jonathan's explanation above of the GIUH). I ran a quick test and the GIUH outputs 6.9% less runoff when running on a 1 h time step versus a 1 s time step given the same total input water (5 mm/h for 24 h). Although a relatively small difference, it's a difference nonetheless.

SnowHydrology commented 2 years ago

I should add that this is not uncommon for hydrologic models. You could force many models to run on non-standard time steps, but you'll get wonky outputs.

andywood commented 2 years ago

Hi Jonathan, Good discussion & nice test, Keith. Non-linear scaling of certain processes is definitely an issue in hydrology. A big challenge for modelers (including our teams) is to develop scalable process parameterizations so that you can calibrate a model at one timestep or spatial resolution and get good performance at another, without changing parameters. Success at this has been spotty.

I think it is generally safe to use implicit assumptions around forcings, since it would be really odd if a forcing did not match an accumulation or rate calculation period to the timestep -- ie give a kg/m2 for 0.5 or 2 hours and put it in a 1-hour slot of a forcing file. We probably should make use of implicit DTs inferred from the forcing files, if they're not explicitly given. Clearly, the unit must be given so that you know if you have a depth or mass that needs to be converted to a rate. Also, the standard density of water assumption for precip forcings is common -- near as I can tell most datasets give liquid water depth w/ density = 1000 kg/m3 or do conversions using that setting internally for non-frozen processes. Since we handle a finite # of input datasets, it's reasonable to check it for each just to make sure that's a valid assumption.

I think it's ok for models to handle fluxes (such as precip) either as timestep accumulations or rates internally, regardless of how they're read in. For adaptive timestepping models, it may be easier if a model converts to L/T (a rate) internally to facilitate sub-stepping. Either way it can be easy to make a mistake, and those are not uncommon in parameterizations. I've seen such bugs, eg for max interception capacity on a canopy -- if that non-scaling for DT and the model is run at 10 min vs 1 hr, you can get much different behavior. We should definitely add documentation in our codes for what they expect, even if it seems duplicative.

Another thing that is critical to check is the definition of a forcing with respect to the timestamp: I've been tripped up by precip written as a forward-looking accumulation or rate (for the period after the time step) when I expected backward looking (for the period before). Temperature can also be instantaneous (at the time stamp) or a forward or backward looking mean timestep temperature. In any case, there are some good examples of full metadata out there, such as NLDAS.

Cheers, -Andy

netcdf NLDAS_FORA0125_H.A20200430.2300.002.2020192183218.pss { dimensions: lat_110 = 224 ; lon_110 = 464 ;

variables: float APCPsfc_110_SFC_acc1h(lat_110,lon_110) ; APCPsfc_110_SFC_acc1h:initial_time = "04/30/2020 (22:00)" ; APCPsfc_110_SFC_acc1h:forecast_time_units = "hours" ; APCPsfc_110_SFC_acc1h:forecast_time = 1 ; APCPsfc_110_SFC_acc1h:model = "MESO ETA Model" ; APCPsfc_110_SFC_acc1h:parameter_number = 61 ; APCPsfc_110_SFC_acc1h:parameter_table_version = 130 ; APCPsfc_110_SFC_acc1h:gds_grid_type = 0 ; APCPsfc_110_SFC_acc1h:level_indicator = 1 ; APCPsfc_110_SFC_acc1h:_FillValue = 1.e+20f ; APCPsfc_110_SFC_acc1h:units = "kg/m^2" ; APCPsfc_110_SFC_acc1h:long_name = "Precipitation hourly total" ; APCPsfc_110_SFC_acc1h:center = "US National Weather Service - NCEP (WMC)" ; APCPsfc_110_SFC_acc1h:sub_center = "NESDIS Office of Research and Applications" ;

On Wed, Nov 17, 2021 at 9:52 AM K. Jennings @.***> wrote:

I should add that this is not uncommon for hydrologic models. You could force many models to run on non-standard time steps, but you'll get wonky outputs.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/NOAA-OWP/cfe/issues/28#issuecomment-971767007, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIKAROTP5NAMRQTWKRTVQDUMPMUZANCNFSM5IFOTODQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

SnowHydrology commented 2 years ago

Coming back to this issue, the original t-shirt/CFE was written with assumed precipitation of mm h-1 (or kg m-2 h-1). It also appears that the time step, in some cases, is assumed to be 1 h. What if we were to multiply the precipitation in the line @hellkite500 linked above by hourly fraction time step?

SnowHydrology commented 2 years ago

Sorry it took me so long to catch this, but is CFE hard-coded to have a 3600 s (1 h) time step?

https://github.com/NOAA-OWP/cfe/blob/e8c2d4b0391cec7d4191beb16f8c10650e3e02e2/src/bmi_cfe.c#L2220

stcui007 commented 2 years ago

It would seem so. This conforms with the aorc forcing time interval we are currently using.

On Thu, Nov 18, 2021 at 1:33 PM K. Jennings @.***> wrote:

Sorry it took me so long to catch this, but is CFE hard-coded to have a 3600 s (1 h) time step?

https://github.com/NOAA-OWP/cfe/blob/e8c2d4b0391cec7d4191beb16f8c10650e3e02e2/src/bmi_cfe.c#L2220

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/NOAA-OWP/cfe/issues/28#issuecomment-973190733, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACA4SRJBX5XZLKPLORXP4YLUMVIHPANCNFSM5IFOTODQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

jmframe commented 2 years ago

But when running a fraction of the specified time step size, the update until function changes it to the fraction: https://github.com/NOAA-OWP/cfe/blob/e8c2d4b0391cec7d4191beb16f8c10650e3e02e2/src/bmi_cfe.c#L1017 then changes it back: https://github.com/NOAA-OWP/cfe/blob/e8c2d4b0391cec7d4191beb16f8c10650e3e02e2/src/bmi_cfe.c#L1020

SnowHydrology commented 2 years ago

So here's the situation as I understand:

It sounds like we can fix by: