Open hellkite500 opened 3 years ago
I believe the original t-shirt code ran all the water fluxes as m
, including precip. Maybe a var (or several vars) got lost in translation at some point. @jmframe do you know?
The printouts are in different units than the model variables.
https://github.com/NOAA-OWP/cfe/blob/3617bda7b887ca283a971cc630fd76d890292db1/original_author_code/t-shirt_0.99f.c#L454 https://github.com/NOAA-OWP/cfe/blob/3617bda7b887ca283a971cc630fd76d890292db1/original_author_code/t-shirt_0.99f.c#L496 https://github.com/NOAA-OWP/cfe/blob/3617bda7b887ca283a971cc630fd76d890292db1/src/bmi_cfe.c#L2438 https://github.com/NOAA-OWP/cfe/blob/3617bda7b887ca283a971cc630fd76d890292db1/src/bmi_cfe.c#L2443
@jmframe Can you clarify this? I.e., does CFE convert the units only when printing output to the terminal window or does it also convert to mm in the course of its execution so that the Q_OUT that gets passed through BMI is in mm?
CFE runs with everything (fluxes and states) based on meters: https://github.com/NOAA-OWP/cfe/blob/3617bda7b887ca283a971cc630fd76d890292db1/src/cfe.c#L211 .
No computations are done in millimeter.
The Q_OUT that gets passed through BMI is in meters: https://github.com/NOAA-OWP/cfe/blob/3617bda7b887ca283a971cc630fd76d890292db1/src/bmi_cfe.c#L183.
There is some code that is included in the CFE model code that prints out converted the states and fluxes (and input) to millimeters. Shown above. I believe this was done for visual clarity when debugging (by Fred).
Thanks @jmframe. @hellkite500, do you have example output in mm/h? I'm also wondering if it could be a time difference (h vs. s can induce the same order of magnitude differences as m vs. mm).
Maybe I've misunderstood, but I thought @hellkite500 was discussing CFE output through Nextgen (i.e., what the model would pass through BMI), not what prints to the screen during the example runs. If that's the case, yes the printed output is definitely in mm, while the model should still pass Q_OUT in m. If that clears up the confusion, I reckon we can close this.
I see. Yeah, the output through Nextgen should be in meters, or whatever conversion is done on the framework side. I'd like to see it, if you care to share.
Using the most up to date bmi-cfe model, the Q_OUT does not show response to the rain fall input. I wonder whether you are using ACPC_surcae or precip_rate as the input. They have vastly different magnitude, and different units as well.
On Wed, Nov 10, 2021 at 3:26 PM Jonathan Frame @.***> wrote:
I see. Yeah, the output through Nextgen should be in meters, or whatever conversion is done on the framework side. I'd like to see it, if you care to share.
— 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/27#issuecomment-965758721, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACA4SRP2MAHQEZJOKJDZ3NTULLPR5ANCNFSM5HWMDATQ . 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.
The CFE Bmi shows that CFE expects precipitation input as kg m-2: https://github.com/NOAA-OWP/cfe/blob/3617bda7b887ca283a971cc630fd76d890292db1/src/bmi_cfe.c#L222. Is that consistent with the precipitation input you are using? This comes from Fred's t-shirt code: https://github.com/NOAA-OWP/cfe/blob/3617bda7b887ca283a971cc630fd76d890292db1/original_author_code/t-shirt_0.99f.c#L434.
@stcui007, as @jmframe noted, CFE expects input precipitation in kg/m2 over its time step. Assuming a liquid water density of 1000 kg/m3 as CFE does means that you can force CFE with mm of precipitation. If your input precipitation is mm/h for a 1 h time step, then all is good. If your input precipitation is mm/s for a 1 h time step, then all is not good unless you convert the rate to a total depth by multiplying by seconds per hour.
@stcui007 and @hellkite500: I just checked the bmi's units in NOAH-MP and CFE, and it seems that unit conversion is necessary! I am using NCAR input, which has the exact units we need to run NOAH-Mp. NOAH-MP outputs are:
QINSUR: total liquid water input to surface rate - m/s
ETRAN: transpiration rate - mm/s
QSEVA: evaporation rate - mm/s
And CFE inputs are;
Atmosphere_water__liquid_equivalent_precipitation_rate - kg/m2 ~ mm/h Water_potential_evaporation_flux - m/s
So both inputs to CFE need to be converted before we get any reasonable results. Water input to surface rate needs to be converted from m/s to mm/h, and potential evapotranspiration from mm/s to m/s.
Thanks Luciana.
Not sure whether we are using the same forcing data. For my forcing input, I believe the BMI-CFE code uses APCP_surface as rain_rate, which is in mm/h. So in bmi_cfe.c code, this is converted to m/h by dividing by 1000 for consistency, which leads to no flow response. Intriguingly, without the division I was getting reasonable streamflow response. We have used this type of forcing data before (almost a year ago) and we were getting reasonable response. As you correctly pointed out, we are getting too much ground water, so no flow above the ground. So something went wrong either in the unit conversion or groundwater control parameters are not used properly.
Shengting
On Mon, Nov 15, 2021 at 11:39 AM lcunha0118 @.***> wrote:
@stcui007 https://github.com/stcui007 and @hellkite500 https://github.com/hellkite500: I just checked the bmi's units in NOAH-MP and CFE, and it seems that unit conversion is necessary! I am using NCAR input, which has the exact units we need to run NOAH-Mp. NOAH-MP outputs are:
QINSUR: total liquid water input to surface rate - m/s ETRAN: transpiration rate - mm/s QSEVA: evaporation rate - mm/s
And CFE inputs are;
Atmosphere_water__liquid_equivalent_precipitation_rate - kg/m2 ~ mm/h Water_potential_evaporation_flux - m/s
So both inputs to CFE need to be converted before we get any reasonable results. Water input to surface rate needs to be converted from m/s to mm/h, and potential evapotranspiration from mm/s to m/s.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NOAA-OWP/cfe/issues/27#issuecomment-969151180, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACA4SROCKHMPPE5ZEM2GDKTUMFAWJANCNFSM5HWMDATQ . 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.
Shengting,
We are using different inputs, since I am using the input generated by NCAR.
This conversion is not sufficient, since NOAH-MP modular requires input rainfall in mm/s. So you need to do 1000/3600 to get the right unit into NOAH-MP modular.
But I am also worried about the potential evpt that is going into CFE. Maybe due to the unit issue, it is too high, and it is just drying everything up before anything gets converted into runoff.
Luciana
Fred developed the CFE model with this example forcing file: https://github.com/NOAA-OWP/cfe/blob/master/original_author_code/cat58_01Dec2015.csv
The BMI for CFE has been tested with the same file and gets similar results as Fred's code. Note that a few changes to the CFE since Fred's version f prevent a perfect match. But they are qualitatively identical.
You can see here that Fred used the APCP_surface to force his model, and he calls if kg m2 (~mm/h): https://github.com/NOAA-OWP/cfe/blob/3617bda7b887ca283a971cc630fd76d890292db1/original_author_code/t-shirt_0.99f.c#L105 https://github.com/NOAA-OWP/cfe/blob/3617bda7b887ca283a971cc630fd76d890292db1/original_author_code/t-shirt_0.99f.c#L967 https://github.com/NOAA-OWP/cfe/blob/3617bda7b887ca283a971cc630fd76d890292db1/original_author_code/t-shirt_0.99f.c#L434
Note that we took Fred's unit conversion: https://github.com/NOAA-OWP/cfe/blob/3617bda7b887ca283a971cc630fd76d890292db1/original_author_code/t-shirt_0.99f.c#L474 and placed it in the update function of the BMI_CFE: https://github.com/NOAA-OWP/cfe/blob/3617bda7b887ca283a971cc630fd76d890292db1/src/bmi_cfe.c#L971
Jonathan, Just to make sure, there is a division by 1000 on line 921 in Initialize function: https://github.com/NOAA-OWP/cfe/blob/3617bda7b887ca283a971cc630fd76d890292db1/src/bmi_cfe.c#L921 Does it cause division by 1000 twice?
On Mon, Nov 15, 2021 at 3:53 PM Jonathan Frame @.***> wrote:
Fred developed the CFE model with this example forcing file:
https://github.com/NOAA-OWP/cfe/blob/master/original_author_code/cat58_01Dec2015.csv
The BMI for CFE has been tested with the same file and gets similar results as Fred's code. Note that a few changes to the CFE since Fred's version f prevent a perfect match. But they are qualitatively identical.
You can see here that Fred used the APCP_surface to force his model, and he calls if kg m2 (~mm/h):
Note that we took Fred's unit conversion:
https://github.com/NOAA-OWP/cfe/blob/3617bda7b887ca283a971cc630fd76d890292db1/original_author_code/t-shirt_0.99f.c#L474 and placed it in the update function of the BMI_CFE:
https://github.com/NOAA-OWP/cfe/blob/3617bda7b887ca283a971cc630fd76d890292db1/src/bmi_cfe.c#L971
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NOAA-OWP/cfe/issues/27#issuecomment-969356294, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACA4SRIDAP66XFIQKMMFJILUMF6MPANCNFSM5HWMDATQ . 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.
That line (921) looks to be unused and can be deleted, because that would then be overwritten by either lines 971 or 975. But it wouldn't cause a second division by 1000, because it doesn't divide the timestep_rainfall_input_m value, it divides the forcing_data_precip_kg_per_m2 value.
Could you run a standalone example for cat-87 and share the output. Hope it won't take too much of your time as you have your daytime job to do. The main issue we are having is not getting a corresponding response to the rain_rate input.in the Q_OUT. Most water seems to have gone to groundwater. If you have any ideas, please let me know. Thanks.
On Tue, Nov 16, 2021 at 11:49 AM Jonathan Frame @.***> wrote:
That line (921) looks to be unused and can be deleted, because that would then be overwritten by either lines 971 or 975. But it wouldn't cause a second division by 1000, because it doesn't divide the timestep_rainfall_input_m value, it divides the forcing_data_precip_kg_per_m2 value.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NOAA-OWP/cfe/issues/27#issuecomment-970516726, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACA4SRMD3ECSG2BKZYWCVLLUMKKTZANCNFSM5HWMDATQ . 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.
cfe_test_nov_16.zip Attached are outputs from the test cases (using the make_and_run*.sh scripts in the repository) You can also see the response in the python version here: https://github.com/NOAA-OWP/cfe/blob/master/py_cfe/run_cfe.ipynb. If most water is going into groundwater, it sounds like a parameter issue. Or, there is not enough precipitation to cause runoff, which could be a unit issue. Maybe check that the values for precipitation that go into the model from the framework match what you would expect?
Thanks, Jonathan.
On Tue, Nov 16, 2021 at 1:45 PM Jonathan Frame @.***> wrote:
cfe_test_nov_16.zip https://github.com/NOAA-OWP/cfe/files/7549288/cfe_test_nov_16.zip Attached are outputs from the test cases (using the make_and_run*.sh scripts in the repository) You can also see the response in the python version here: https://github.com/NOAA-OWP/cfe/blob/master/py_cfe/run_cfe.ipynb. If most water is going into groundwater, it sounds like a parameter issue. Or, there is not enough precipitation to cause runoff, which could be a unit issue. Maybe check that the values for precipitation that go into the model from the framework match what you would expect?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NOAA-OWP/cfe/issues/27#issuecomment-970618290, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACA4SRPKNNPPTDOGYWVXZ2DUMKX37ANCNFSM5HWMDATQ . 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.
@hellkite500, @jmframe, @stcui007, @lcunha0118, can this issue be closed? Q_OUT
should definitely be m per time step.
Looking at some of the outputs of CFE model runs, it almost looks like the Q_OUT variable is in
mm/hr
, much the same as theprecip_rate
is. But the BMI reports Q_OUT as a unit ofm
. When assuming this unit and converting the value tom^3/s
using the catchment area, the resulting flow values are orders of magnitude higher than expected. This also points to the units declared for this variable as being inconsistent with the units the underlying formulation is using for this variable.