Add support for NWM 3.0 baseline output variables #86

GreyREvenson · 2023-09-15T14:46:40Z

Purpose

The purpose of this PR is to expose Noah-OM variables via BMI to replicate the functionality of NWM 3.0.

Additions

The following model variables were added to output_items in bmi/bmi_noahowp.f90:

ACSNOM -- The accumulated meltwater from bottom snow layer (mm) (NWM 3.0 output variable)
SNOWT_AVG -- The average snow temperature (K) (by layer mass) (NWM 3.0 output variable)
ISNOW -- The number of snow layers (unitless) (NWM 3.0 output variable)
QRAIN -- The rainfall rate on the ground (mm/s) (NWM 3.0 output variable)
FSNO -- The snow-cover fraction on the ground (unitless fraction) (NWM 3.0 output variable)
SNOWH -- The snow depth (m) (NWM 3.0 output variable)
SNLIQ -- The snow layer liquid water (mm) (NWM 3.0 output variable)
QSNOW -- The snowfall rate on the ground (mm/s) (NWM 3.0 output variable)
ECAN -- Evaporation of intercepted water (mm) (NWM 3.0 output variable)
GH -- The heat flux into the soil (W/m-2) (NWM 3.0 output variable)
TRAD -- The surface radiative temperature (K) (NWM 3.0 output variable)
FSA -- The total absorbed SW radiation (W/m-2) (NWM 3.0 output variable)
CMC -- The total canopy water (liquid + ice) (mm) (NWM 3.0 output variable)
LH -- The total latent heat to the atmosphere (W/m-2) (NWM 3.0 output variable)
FIRA -- The total net LW radiation to atmosphere (W/m-2) (NWM 3.0 output variable)
FSH -- The total sensible heat to the atmosphere (W/m-2) (NWM 3.0 output variable)

Listings for the above variables were added to the following bmi/bmi_noahowp.f90 subroutines:

noahowp_var_grid
- All variables were assigned to the rank 2 grid, except for SNLIQ, for which a new instance of GridType (in bmi/bmi_grid.f90) was initialized with rank 3
noahowp_grid_type
- Added GridType%id = 2 (GridType%rank = 3) to uniform_rectilinear listing so that noahowp_grid_type works for SNLIQ and other 3D variables
- All other variables should work as-is
noahowp_var_type
- All variable added to listing for real except of ISNOW, which was added as integer
noahowp_var_units
- Some variables added and some variables have a units change
noahowp_var_itemsize
noahowp_get_int
- (Only for ISNOW)
noahowp_get_float
- (All except ISNOW)

ACSNOM was added to the watergrid_type and water_type derived data types as well as the WaterVarInTransfer and WaterVarOutTransfer subroutines. Added calculation of ACSNOM in WaterMain subroutine in src/WaterModule.f90

SNOWT_AVG was added to the energygrid_type and energy_type derived data types as well as the EnergyVarInTransfer and EnergyVarOutTransfer subroutines. Added calculation of SNOWT_AVG in the SnowWater subroutine in SnowWaterModule.f90.

Changes

This PR adds an additional instance of GridType to the bmi_noahowp derived type definition in /bmi/bmi_noahowp.f90. The additional instance corresponds to GridType%rank = 3 and was needed because SNLIQ was added as a BMI-exposed variable and is 3D.

The PR modifies the BMI test program (i.e., noahmp_driver_test in /test/noahowp_driver_test.f90) to work the the newly added output variables. Modifications were necessary largely because the newly added SNLIQ variable is 3D and therefore needed to be handled differently than the other variables.

Testing

I re-compiled and re-executed /test/noahowp_driver_test.f90 and got these results, which appear correct.

I also re-compiled and re-executed /run/noah_owp_modular.exe and got these results, which are correct given that cell (1,1) has the same input values as listed in namelist.input of the main branch of this repo.

Notes

SNLIQ's output_grid value should be 2 (i.e., rank 2). output_grid is currently set to 1 where output_grid is defined. I set output_grid(14) = 2 where the value 14 corresponds to SNLIQ.

GreyREvenson · 2023-09-27T14:03:58Z

@nmizukami, would you have time available to review this PR in the near future?

GreyREvenson · 2023-09-27T14:05:55Z

@hellkite500: FYI I added another instance of GridType for rank 3 grids in this PR. Just calling your attention to this in case you'd like to check that I implemented correctly.

nmizukami · 2023-09-27T14:57:40Z

@nmizukami, would you have time available to review this PR in the near future?

yes, I can do review!

hellkite500 · 2023-09-27T17:55:44Z

@hellkite500: FYI I added another instance of GridType for rank 3 grids in this PR. Just calling your attention to this in case you'd like to check that I implemented correctly.

Took a quick peek -- looks like the grid module is doing what I had hoped! But I'll let you tell me whether or not adding that and using those mechanics was intuitive/simple enough or if it was the opposite!

nmizukami

Hi @GreyEvenson-NOAA,

Just looking through the PR. I think overall it looks good to me and I added several picky comments! Not super important but something to think about.

nmizukami · 2023-09-29T14:32:32Z

bmi/bmi_noahowp.f90

+    output_items(20) = 'CMC'       ! Total canopy water (liquid + ice) (mm) (NWM 3.0 output variable)
+    output_items(21) = 'LH'        ! Total latent heat to the atmosphere (W/m-2) (NWM 3.0 output variable)
+    output_items(22) = 'FIRA'      ! Total net LW radiation to atmosphere (W/m-2) (NWM 3.0 output variable)
+    output_items(23) = 'FSH'       ! Total sensible heat to the atmosphere (W/m-2) (NWM 3.0 output variable)


Nice to see the comments on each variables!

nmizukami · 2023-09-29T14:35:29Z

bmi/bmi_noahowp.f90

      units = "mm"
      bmi_status = BMI_SUCCESS
+   case("FSNO","ISNOW")
+      units = "1"


What do you mean by units=1?? I know it is unitless so maybe just say unitless?

I changed this to units = "unitless"

nmizukami · 2023-09-29T14:37:20Z

bmi/bmi_noahowp.f90

+      bmi_status = BMI_SUCCESS
+   case("SNOWH")
+      units = "m"
+      bmi_status = BMI_SUCCESS
   case default
      units = "-"


So here, units is "-", which I assume is unitless. maybe so use "-" for FSNO and ISNOW units?

I'm actually not certain if "-" is meant to mean unitless as I'm thinking that the default case is intended to be used when noahowp_var_units is called for a variable name that isn't in input_items or output_items (?). So I was thinking we'd leave this as-is but make a change for FSNO and ISNOW. Let me know if you see potential problems in this approach.

nmizukami · 2023-09-29T14:43:58Z

bmi/bmi_noahowp.f90

      bmi_status = BMI_SUCCESS
   case("QSEVA")
-      dest = reshape(watergrid%qseva,[n_x*n_y])
+      dest = reshape(watergrid%qseva*1000.,[n_x*n_y])


is 1000 for unit conversion m to mm (I assume)? maybe consider creating unit conversion variables like instead of using magic numbers?

real, parameter :: mm2m = 0.001 ! unit conversion mm to m real, parameter :: m2mm = 1000. ! unit conversion m to mm

I added mm2m and m2mm as local conversion factors rather than declaring them as parameters (see my separate comment explaining why I did this).

I changeed this line to: dest = reshape(watergrid%qseva*m2mm,[n_x*n_y])

nmizukami · 2023-09-29T14:44:16Z

bmi/bmi_noahowp.f90

      bmi_status = BMI_SUCCESS
   case("EVAPOTRANS")
-      dest = reshape(watergrid%evapotrans,[n_x*n_y])
+      dest = reshape(watergrid%evapotrans*domaingrid%DT*0.001,[n_x*n_y])


the same for previous comment

I changed this listing to dest = reshape(watergrid%evapotrans*domaingrid%DT*mm2m,[n_x*n_y])

nmizukami · 2023-09-29T14:48:26Z

src/WaterGridType.f90

  real,allocatable,dimension(:,:)                             :: PONDING
  real,allocatable,dimension(:,:)                             :: PONDING1
  real,allocatable,dimension(:,:)                             :: PONDING2


maybe add comments on these three variables? so it looks complete in the whole list of the declarations

I added comments for these variables such that these lines were revised to:

real,allocatable,dimension(:,:) :: PONDING ! surface ponding [mm] from snowmelt when thin snow has no layer real,allocatable,dimension(:,:) :: PONDING1 ! surface ponding [mm] from thin snow liquid during layer combination real,allocatable,dimension(:,:) :: PONDING2 ! surface ponding [mm] from thin snow liquid during transition from multilayer to no layer

These variable descriptions came from page 263 of He, C., Valayamkunnath, P., Barlage, M., Chen, F., Gochis, D., Cabell, R., … Ek, M. (2023). The Community Noah-MP Land Surface Modeling System Technical Description Version 5.0 (No. NCAR/TN-575+STR). doi:10.5065/ew8g-yr95

nmizukami · 2023-09-29T14:49:24Z

src/WaterModule.f90

@@ -108,6 +108,8 @@ SUBROUTINE WaterMain (domain, levels, options, parameters, forcing, energy, wate
    ! convert units (mm/s -> m/s)
    !PONDING: melting water from snow when there is no layer
    water%QINSUR = (water%PONDING+water%PONDING1+water%PONDING2)/domain%DT * 0.001


here also can use unit conversion constants

I added mm2m and m2mm as local variables rather than declaring them parameters (see separate comment on this matter).

nmizukami · 2023-09-29T15:10:14Z

test/noahowp_driver_test.f90

-        if(.NOT.allocated(grid_temp_real)) allocate(grid_temp_real(n_x,n_y))
-        if(size(var_value_get_real).ne.grid_size) then
-          deallocate(var_value_get_real)
+      print*,'    rank = ',grid_rank


So here, there are mostly just print statement (so no formatted writing), but there is one write statement with format, and looking at test output (noahowp_test_output.txt).

EVAPOTRANS rank = 2 column count (n_x) = 2 row count (n_y) = 3 get_value function cpu time (s) = 1.90734863E-06 from get_value (flattened) = 0.00000000 0.00000000 0.00000000 0.00000000 8.58270573E-12 0.00000000 from get_value (in XY) = 0.0 0.0 0.0 0.0 0.0 0.0

there is small number 8.58270573E-12 in flatted output, but 0.0 in gridded output.

Wonder if formatted output (give more precision than F10.1 ) should be used for everything here?

I revised /test/noahowp_driver_test.f90 write statement formatting (here and throughout) per your suggestion. Let me know if you think the formatting could be improved further.

Regarding EVAPOTRANS specifically, the revised program writes:

EVAPOTRANS rank = 2 column count (n_x) = 2 row count (n_y) = 3 get_value function cpu time (s) = 0.00000000 from get_value (flattened) = 0.00000 0.00000 0.00000 0.00000 0.00000 0.00000 from get_value (in XY) = 0.00000 0.00000 0.00000 0.00000 0.00000 0.00000 our replacement value (flattened) = 1.00000 2.00000 3.00000 4.00000 5.00000 6.00000 our replacement value (in XY) = 1.00000 2.00000 3.00000 4.00000 5.00000 6.00000 set_value function cpu time (s) = 1.90734863E-06 and the new value (flattened) = 1.00000 2.00000 3.00000 4.00000 5.00000 6.00000 and the new value (in XY) = 1.00000 2.00000 3.00000 4.00000 5.00000 6.00000

GreyREvenson · 2023-10-03T16:37:49Z

Thank you, @nmizukami! Appreciate you taking the time. I'm out of town until Wednesday so will get on this on Thursday. Thanks again!

This reverts commit 44fe19f.

This reverts commit f30cdf4.

GreyREvenson · 2023-10-06T15:29:00Z

Regarding the conversion factor variables m2mm and mm2m:

The compiler did not allow me to declare m2mm and mm2m as parameter in both the bminoahowp and WaterModule modules because bminoahowp uses WaterModule (via its use of RunModule).

The correct thing to do would seem to be to add m2mm and mm2m to ConstantsModule and use it in bminoahowp and WaterModule. However, I held off on that as I thought this would be better addressed by a separate PR for issue #5.

Rather than adding m2mm and mm2m as parameters, I declared them as local variables within the scope of the noahowp_get_float, noahowp_set_float, and WaterMain subroutines (where this PR uses those conservation factors).

Let me know if you think we should approach this a different way or if you see a better solution short of implementing ConstantsModule.

GreyREvenson · 2023-10-06T16:12:47Z

@nmizukami, I believe we're ready for you to have another look. I appreciate your time and input!

FYI: I updated the results for /test/noahowp_driver_test.f90 and /run/noah_owp_modular.exe in the TESTING section of this PR's title/first comment.

nmizukami · 2023-10-09T12:38:14Z

bmi/bmi_noahowp.f90

@@ -1194,7 +1197,7 @@ function noahowp_set_float(this, name, src) result (bmi_status)
      watergrid%etran = reshape(src/domaingrid%DT,[n_x,n_y])
      bmi_status = BMI_SUCCESS
   case("QSEVA")
-      watergrid%qseva = reshape(src,[n_x,n_y])
+      watergrid%qseva = reshape(src*mm2m,[n_x,n_y])


good to catch a small bug!?

Yup indeed.

nmizukami

Thank you @GreyEvenson-NOAA. It looks good to me.

For the constant variables, yes, another PR to deal sounds good.

For the format, test result looks better now, but you might also want to look into G format (I think it is more general), but for this PR, i think it is ok as is (if you find G-format better and it easy to implement it, would go ahead and change now)

GreyREvenson · 2023-10-11T12:57:04Z

Thank you @GreyEvenson-NOAA. It looks good to me.

For the constant variables, yes, another PR to deal sounds good.

For the format, test result looks better now, but you might also want to look into G format (I think it is more general), but for this PR, i think it is ok as is (if you find G-format better and it easy to implement it, would go ahead and change now)

Thanks for the tip! I implemented the G format codes per your suggestion. Nice and simple.

GreyEvenson-NOAA added 3 commits September 7, 2023 14:30

Uploading notes

9c39734

Cleanup

955c682

Added support for SNOWT_AVG (a NWM 3.0 baseline output variable)

aad8a98

GreyREvenson marked this pull request as draft September 15, 2023 14:46

GreyEvenson-NOAA added 11 commits September 18, 2023 15:43

Added calculation/setting of GH variable

ee3057d

Adding support for NWM baseline output variables

2c8311e

Add ACSNOM to watergrid_type and water_type

a9a7ce9

Revise calculation of SNOWT_AVG

9958215

Set shape differently for rank 3 grids

9f15d03

Revise for newly exposed variables

4b8f8a8

Revise for newly exposed output variables

1705821

Revise noahowp_var_itemsize listing for ACSNOM

7019a64

Cleanup

b7daa1b

Cleanup

4be4820

Cleanup

60c0bd0

GreyREvenson marked this pull request as ready for review September 27, 2023 14:02

GreyREvenson requested a review from nmizukami September 27, 2023 14:06

nmizukami reviewed Sep 29, 2023

View reviewed changes

GreyEvenson-NOAA added 7 commits October 5, 2023 10:47

Add comment descriptions for PONDING, PONDING1 and PONDING2 variables

cbeaaf8

Revisions per PR feedback

a7b0e17

Revised module to use newly added unit conversion parameters

a70d725

Update makefiles to include ConstantsModule.f90

44fe19f

Adding mm -> m and m -> mm conversion parameters

f30cdf4

Now using constants module

b73625c

Use constants module and cleanup

2ed1e5b

GreyEvenson-NOAA added 5 commits October 5, 2023 17:08

Update test program formatting and organization

ccbadfc

Revert "Update makefiles to include ConstantsModule.f90"

45e41d6

This reverts commit 44fe19f.

Revert "Adding mm -> m and m -> mm conversion parameters"

51dabe9

This reverts commit f30cdf4.

Add local conversion factor variables

88b57b2

Update formatting

55ad959

Updated notes

fcbb498

GreyREvenson requested a review from nmizukami October 6, 2023 16:11

nmizukami reviewed Oct 9, 2023

View reviewed changes

nmizukami approved these changes Oct 9, 2023

View reviewed changes

Implement 'G' formatting codes

0c702e2

GreyREvenson merged commit afa54c5 into NOAA-OWP:noah_om_grid Oct 11, 2023

GreyREvenson deleted the noah_om_grid_nwm_baseline_vars branch October 11, 2023 13:00

GreyREvenson mentioned this pull request Nov 6, 2023

Add model mask functionality and gridded attributes for test basin #92

Merged

GreyREvenson mentioned this pull request Nov 29, 2023

Patch SNOWT_AVG and ACSNOM #94

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for NWM 3.0 baseline output variables #86

Add support for NWM 3.0 baseline output variables #86

GreyREvenson commented Sep 15, 2023 •

edited

Loading

GreyREvenson commented Sep 27, 2023

GreyREvenson commented Sep 27, 2023

nmizukami commented Sep 27, 2023

hellkite500 commented Sep 27, 2023

nmizukami left a comment

nmizukami Sep 29, 2023

nmizukami Sep 29, 2023 •

edited

Loading

GreyREvenson Oct 5, 2023

nmizukami Sep 29, 2023 •

edited

Loading

GreyREvenson Oct 5, 2023

nmizukami Sep 29, 2023 •

edited

Loading

GreyREvenson Oct 5, 2023 •

edited

Loading

nmizukami Sep 29, 2023

GreyREvenson Oct 5, 2023

nmizukami Sep 29, 2023

GreyREvenson Oct 5, 2023

nmizukami Sep 29, 2023

GreyREvenson Oct 5, 2023 •

edited

Loading

nmizukami Sep 29, 2023 •

edited

Loading

GreyREvenson Oct 6, 2023

GreyREvenson commented Oct 3, 2023

GreyREvenson commented Oct 6, 2023 •

edited

Loading

GreyREvenson commented Oct 6, 2023 •

edited

Loading

nmizukami Oct 9, 2023

GreyREvenson Oct 11, 2023

nmizukami left a comment •

edited

Loading

GreyREvenson commented Oct 11, 2023

Add support for NWM 3.0 baseline output variables #86

Add support for NWM 3.0 baseline output variables #86

Conversation

GreyREvenson commented Sep 15, 2023 • edited Loading

Purpose

Additions

Changes

Testing

Notes

GreyREvenson commented Sep 27, 2023

GreyREvenson commented Sep 27, 2023

nmizukami commented Sep 27, 2023

hellkite500 commented Sep 27, 2023

nmizukami left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmizukami Sep 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmizukami Sep 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmizukami Sep 29, 2023 • edited Loading

Choose a reason for hiding this comment

GreyREvenson Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GreyREvenson Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

nmizukami Sep 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GreyREvenson commented Oct 3, 2023

GreyREvenson commented Oct 6, 2023 • edited Loading

GreyREvenson commented Oct 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmizukami left a comment • edited Loading

Choose a reason for hiding this comment

GreyREvenson commented Oct 11, 2023

GreyREvenson commented Sep 15, 2023 •

edited

Loading

nmizukami Sep 29, 2023 •

edited

Loading

nmizukami Sep 29, 2023 •

edited

Loading

nmizukami Sep 29, 2023 •

edited

Loading

GreyREvenson Oct 5, 2023 •

edited

Loading

GreyREvenson Oct 5, 2023 •

edited

Loading

nmizukami Sep 29, 2023 •

edited

Loading

GreyREvenson commented Oct 6, 2023 •

edited

Loading

GreyREvenson commented Oct 6, 2023 •

edited

Loading

nmizukami left a comment •

edited

Loading