gmtb/develop: address physics issue 300 and coupled code with bugfix #308

climbfuji · 2019-08-28T17:18:07Z

This PR combines PRs address issue 300 (#302) from @grantfirl and add capability to run in coupled configuration (#299) from @panll, with bugfixes from @climbfuji for PR #299.

See https://github.com/NCAR/FV3/pull/195 for regression testing information.

Gmtb/develop

…he code for RUC LSM); change units of soil moisture content and runoff variables to kg m-2 (CCPP metadata had been in error and they only were in kg m-2 after a conversion in GFS_diagnostics.F90); should be merged with removal of conversion factor in GFS_diagnostics.F90 in FV3 repo

modified according to Dom's comments passed regression test.

modified sfc_cice.f

…into HEAD

…sics into HEAD

… correct logic for cice land-sea-ice mask

climbfuji · 2019-08-28T17:43:18Z

Associated PRs:

https://github.com/NCAR/FV3/pull/195
#308

Tested together with NCAR/NEMS#3 and https://github.com/NCAR/NEMSfv3gfs/pull/234

grantfirl · 2019-08-28T22:29:38Z

physics/sfc_sice.f

@@ -263,28 +261,19 @@ subroutine sfc_sice_run                                           &
      errflg = 0

      if(cplflx)then
-        write(*,*)'Fatal error: CCPP is not ready for cplflx=true!!'
+        write(*,*)'Fatal error: CCPP not been tested with cplflx=true!'


Do we still need this here after this PR? Until the NEMSCompset changes are made to work with CCPP?

Yes, we agreed to keep this in here until we have actually tested it on our end in a coupled configuration.

grantfirl · 2019-08-28T22:32:18Z

physics/sfc_sice.f

-            endif
-          enddo
-      endif
+      if (cplflx) then


I like this syntax. It makes sense to me.

Thanks! Very pythonic and easily readable.

grantfirl · 2019-08-28T22:38:16Z

physics/GFS_surface_generic.F90

+          hice_cice(i)   = hice(i)
+          if(flag_cice(i)) tsfc(i) = fice_cice(i)*tisfc_cice(i) + (1. - fice_cice(i))*tsea_cice(i)
+        enddo
+      else


flag_cice is already set to F in interstitial_phys_reset, so this isn't necessary, although it doesn't hurt.

You are correct. Same for line 266 just above. I can remove lines 265-266 and lines 279-281 if you prefer (I'll repeat a small set of tests, e.g. rt_ccpp_gsd in verify mode).

I removed those lines

grantfirl · 2019-08-28T22:46:17Z

physics/GFS_PBL_generic.F90

+!! | q1                           | water_vapor_specific_humidity_at_lowest_model_layer_for_diag                      | layer 1 specific humidity for diag                                                          | kg kg-1       |    1 | real      | kind_phys | in     | F        |
+!! | prsl                         | air_pressure                                                                      | mean layer pressure                                                                         | Pa            |    2 | real      | kind_phys | in     | F        |
+!! | hflx                         | kinematic_surface_upward_sensible_heat_flux                                       | kinematic surface upward sensible heat flux                                                 | K m s-1       |    1 | real      | kind_phys | in     | F        |
+!! | ushfsfci                     | instantaneous_upward_sensible_heat_flux                                           | instantaneous upward sensible heat flux                                                     | W m-2         |    1 | real      | kind_phys | out    | F        |


This is very close to the other standard name 'instantaneous_surface_upward_sensible_heat_flux'. Other variables in the Coupling DDT that have similar names (or represent the same physical quantities) as those in other DDTs have been distinguished by appending "for_coupling" to the end. So, maybe consider "instantaneous_surface_upward_sensible_heat_flux_for_coupling" for a standard name (also change in GFS_typedefs metadata).

But dtsfci_cpl already has the standard name instantaneous_surface_upward_sensible_heat_flux_for_coupling (also present in the same metadata table)

What shall we do with this? Keep the "dangerous" name or decorate with something else?

I wonder if you can hear my sigh through the internet. :)

So many variables for the same thing with tiny nuances. In this instance, I cannot tell at first glance the difference between dtsfci_cpl and ushfsfci. Different sign? Looks like ushfsfci is only used for chemistry coupling. Could we add "for_chemistry_coupling" instead?

It would be really nice to have a written procedure for this.

Ok I will do this. You should be glad that you can't hear my cursing through the internet!

grantfirl · 2019-08-28T22:58:08Z

physics/GFS_PBL_generic.F90

+          tem  = prsl(i,1) / (rd*t1(i)*(1.0+fvirt*tem1))
+          ushfsfci(i) = -cp * tem * hflx(i) ! upward sensible heat flux
+        enddo
+!!        Coupling%dkt     (:,:) = dkt (:,:)


How can we get away with not setting this? dkt is an interstitial and is reset every time step, whereas Coupling%dkt is not.

I would ask the original author as to why. But noting that Interstitial%dkt has dimensions (IM,Model%levs-1), while Coupling%dkt has (IM,Model%levs). Hence, we'd have to make this line

dkt_cpl(1:im,1:levs-1) = dkt(1:im,1:levs-1)

I can make this change as well.

I made this change.

Maybe we can only get away with it because cplchm has not been turned on? @panll

Already done.

grantfirl · 2019-08-28T23:05:15Z

physics/GFS_surface_generic.F90

+          dvsfc_cice(i)  = dvsfcin_cpl(i)
+          dtsfc_cice(i)  = dtsfcin_cpl(i)
+          dqsfc_cice(i)  = dqsfcin_cpl(i)
+          tisfc_cice(i)  = tisfc(i)


I don't see the equivalent of lines 273-276 in GFS_physics_driver.F90 anymore. Are they still needed?

I removed them, can't find any reference to this code in GFS_physics_driver.F90 either.

…titial dkt to coupling dkt

… no longer in GFS_physics_driver.F90, remove redundant assignment of default values for flag_cice

…t_flux to instantaneous_surface_upward_sensible_heat_flux_for_chemistry_coupling

grantfirl

Looks good to me now after @climbfuji's changes. Thanks @climbfuji and @panll

climbfuji · 2019-08-28T23:40:26Z

Looks good to me now after @climbfuji's changes. Thanks @climbfuji and @panll

Thanks. I will rerun the rt_ccpp_gsd.conf verify-mode tests and the rt_ccpp_gmtb.conf verify-mode tests. This should be sufficient evidence that the code still gives the same answer.

Features: - Implemented sharing of atmospheric tracer metadata (name, units) to coupled model component via a standard NUOPC connector if coupling tracer field is accessed by shared memory reference - Introduced additional metadata to tracer records in field_table to identify and validate prognostic and diagnostic chemistry tracers. A new record line starting with the `tracer_usage` keyword is now required to recognize chemistry tracer as follows: "tracer_usage", "chemistry" (default, prognostic) "tracer_usage", "chemistry", "type=diagnostic" (diagnostic) - Added sanity check for input chemistry tracers, ensuring that both prognostic and diagnostic tracers in field_table are contiguous, and diagnostic tracers follow prognostic ones. - Improved algorithm to set aerosol scavenging factors - Exported instantaneous fields for: - 3D non-convective liquid and ice precipitation fluxes - 3D cloud fraction - normalized soil wetness - ice fraction (as used in the atmospheric model) - lake fraction - ocean fraction - surface snow area fraction - Enabled import of diagnostic tracers Fixes: - Prevent a divide by zero error when constant arrays are rescaled for lossy compression in `write_netcdf_parallel()` - Correctly sample and compute elapsed time for field bundle regridding operations. - Include hotfix (see PR NCAR#308) to allow latest `setup_exportdata()` to work with coupled chemistry. Co-authored-by: Dom Heinzeller <[email protected]>

linlin-pan and others added 10 commits August 15, 2019 09:05

adding coupling option in ccpp

307769d

Merge pull request #2 from NCAR/gmtb/develop

f280afc

Gmtb/develop

adding coupling option to CCPP

f76289e

adding coupling option in ccpp

a82db03

modified according to Dom's comments passed regression test.

adding coupling option to CCPP

9b56b90

modified sfc_cice.f

Merge branch 'gmtb/develop' of https://github.com/panll/ccpp-physics …

91ae366

…into HEAD

Merge branch 'fix_LSM_units' of https://github.com/grantfirl/ccpp-phy…

a9f5ee1

…sics into HEAD

physics/GFS_PBL_generic.F90: formatting changes, updates of comments

a72e150

physics/sfc_cice.f, physics/sfc_sice.f: bugfixes for coupling code to…

da8bea5

… correct logic for cice land-sea-ice mask

climbfuji marked this pull request as ready for review August 28, 2019 19:36

climbfuji requested review from grantfirl and llpcarson as code owners August 28, 2019 19:36

climbfuji mentioned this pull request Aug 28, 2019

Bugfix for NemsCompset detection of Theia/Hera NCAR/NEMS#3

Merged

grantfirl reviewed Aug 28, 2019

View reviewed changes

climbfuji added 3 commits August 28, 2019 17:17

physics/GFS_PBL_generic.F90: bugfix, add missing assignment of inters…

5a21626

…titial dkt to coupling dkt

physics/GFS_surface_generic.F90: bugfix, remove coupling code that is…

3f88590

… no longer in GFS_physics_driver.F90, remove redundant assignment of default values for flag_cice

physics/GFS_PBL_generic.F90: rename instantaneous_upward_sensible_hea…

65ec1e3

…t_flux to instantaneous_surface_upward_sensible_heat_flux_for_chemistry_coupling

grantfirl approved these changes Aug 28, 2019

View reviewed changes

climbfuji merged commit d858b98 into NCAR:gmtb/develop Aug 30, 2019

climbfuji mentioned this pull request Nov 18, 2019

Wrong units for several LSM-related variables #300

Closed

climbfuji deleted the github_gmtb_develop_address_physics_issue_300_and_coupled_code_with_bugfix branch June 27, 2022 03:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gmtb/develop: address physics issue 300 and coupled code with bugfix #308

gmtb/develop: address physics issue 300 and coupled code with bugfix #308

climbfuji commented Aug 28, 2019 •

edited

Loading

climbfuji commented Aug 28, 2019 •

edited

Loading

grantfirl Aug 28, 2019

climbfuji Aug 28, 2019

grantfirl Aug 28, 2019

climbfuji Aug 28, 2019

grantfirl Aug 28, 2019

climbfuji Aug 28, 2019

climbfuji Aug 28, 2019

grantfirl Aug 28, 2019

climbfuji Aug 28, 2019

climbfuji Aug 28, 2019

grantfirl Aug 28, 2019

climbfuji Aug 28, 2019

grantfirl Aug 28, 2019 •

edited

Loading

climbfuji Aug 28, 2019

climbfuji Aug 28, 2019

grantfirl Aug 28, 2019 •

edited

Loading

climbfuji Aug 28, 2019

grantfirl Aug 28, 2019

climbfuji Aug 28, 2019

grantfirl left a comment

climbfuji commented Aug 28, 2019

gmtb/develop: address physics issue 300 and coupled code with bugfix #308

gmtb/develop: address physics issue 300 and coupled code with bugfix #308

Conversation

climbfuji commented Aug 28, 2019 • edited Loading

climbfuji commented Aug 28, 2019 • 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

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

grantfirl Aug 28, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grantfirl Aug 28, 2019 • 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

grantfirl left a comment

Choose a reason for hiding this comment

climbfuji commented Aug 28, 2019

climbfuji commented Aug 28, 2019 •

edited

Loading

climbfuji commented Aug 28, 2019 •

edited

Loading

grantfirl Aug 28, 2019 •

edited

Loading

grantfirl Aug 28, 2019 •

edited

Loading