Merge GFDL new dynamic core to EMC fork/branch #15

XiaqiongZhou-NOAA · 2020-02-13T19:58:58Z

To merge GFDL new dynamic core 201912 release version to EMC fork/branch

Related pull requests:
fv3atm #65
universe-weather-model #58
ccpp-physics #397

climbfuji

Holy cow, quite a number of changes. Looks ok to my limited understanding of the dycore. I was looking specifically for changes with impact on the CCPP. I left a comment on the MULTI_GASES option and a few requests for cosmetic changes.

climbfuji · 2020-05-13T14:48:13Z

driver/fvGFS/atmosphere.F90

@@ -112,7 +112,7 @@ module atmosphere_mod
 !   <tr>
 !     <td>mpp_mod</td>
 !     <td>mpp_error, stdout, FATAL, NOTE, input_nml_file, mpp_root_pe,
-!                    mpp_npes, mpp_pe, mpp_chksum,mpp_get_current_pelist,
+!                    mpp_npes, mpp_pe, mpp_chksum,mpp_get_current_pelist,     


Would you mind removing the trailing whitespaces that are added by this commit?

Dom:
Is this the only place to revise about the trailing whitespaces?
I noticed that there are many "^M" hiding in the file fv_regional_bc.F90 at the end of each line. Is it matter?

^M will be removed.

climbfuji · 2020-05-13T14:50:19Z

driver/fvGFS/atmosphere.F90

-     call fill_gfs(blen, npz, IPD_Data(nb)%Statein%prsi, IPD_Data(nb)%Stateout%gq0, 1.e-9_kind_phys)
+     if (Atm(n)%flagstruct%fill_gfs) call fill_gfs(blen, npz, IPD_Data(nb)%Statein%prsi, IPD_Data(nb)%Stateout%gq0, 1.e-9_kind_phys)
+
+!LMH 28sep18: If the name of a tracer ends in 'nopbl' then do NOT update it;


I wonder if this feature is/will be used by the UFS and if this is documented somewhere.

This is the part added when merge the master branch of GFDL dycore. Lucas could know it better.

climbfuji · 2020-05-13T14:53:24Z

model/fv_control.F90

+       ! Read FVCORE namelist 
+       read (f_unit,fv_core_nml,iostat=ios)
+       ierr = check_nml_error(ios,'fv_core_nml')
+       call close_file(f_unit)
 #ifdef MULTI_GASES


Since we do not have regression tests with MULTI_GASES turned on, has someone tested this feature manually. In particular of the CCPP version works as expected?

@HenryJuang-NOAA

Multi_GASES was tested for this version with 64-bits.
Another pull requests is generated to the regression test ufs-community/ufs-weather-model#112. Some fixes are needed for 32-bits runs in that pull request.

climbfuji · 2020-05-13T15:02:26Z

model/fv_nesting.F90

-      jsd = bd%jsd
-      jed = bd%jed
-
+


I would remove the extra whitespaces in lines 215, 218, 227.

bensonr

Please see the inlined comments regarding multi-gases namelist reads.

bensonr · 2020-05-14T14:41:35Z

model/fv_control.F90

+#ifdef MULTI_GASES
+   namelist /multi_gases_nml/ rilist,cpilist
+#endif


This new version of fv_control has each namelist read contained within its own programmatic unit. Please create a separate subroutine for the multi-gases nml read and add a #ifdef gated call in the proper place.

Can this be changed in the next pull request when we add the regression test for MULTI_GASES?

bensonr · 2020-05-14T14:45:53Z

model/fv_control.F90

@@ -738,26 +1067,12 @@ subroutine run_setup(Atm, dt_atmos, grids_on_this_pe, p_split)
      read (input_nml_file,multi_gases_nml,iostat=ios)
      ierr = check_nml_error(ios,'multi_gases_nml')
 #endif


See comment above regarding separate programmatic unit for multi-gases nml.

HenryJuang-NOAA · 2020-05-14T15:33:20Z

There are many *_nml included in input.nml, so all *_nml use the same unit number through input.nml. I don't see any reason to have one of them doing something different from others....

…

On Thu, May 14, 2020 at 11:04 AM bensonr ***@***.***> wrote: ***@***.**** requested changes on this pull request. Please see the inlined comments regarding multi-gases namelist reads. ------------------------------ In model/fv_control.F90 <#15 (comment)> : > +#ifdef MULTI_GASES + namelist /multi_gases_nml/ rilist,cpilist +#endif This new version of fv_control has each namelist read contained within its own programmatic unit. Please create a separate subroutine for the multi-gases nml read and add a #ifdef gated call in the proper place. ------------------------------ In model/fv_control.F90 <#15 (comment)> : > @@ -738,26 +1067,12 @@ subroutine run_setup(Atm, dt_atmos, grids_on_this_pe, p_split) read (input_nml_file,multi_gases_nml,iostat=ios) ierr = check_nml_error(ios,'multi_gases_nml') #endif See comment above regarding separate programmatic unit for multi-gases nml. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALRWWEPDW5R6RXTNCCPVQLDRRQB7RANCNFSM4KUZZYTA> .

HenryJuang-NOAA · 2020-05-14T15:38:08Z

I see, sorry! Each one has its own unit. Well! What is the good reason to have each nml have its own unit?

…

On Thu, May 14, 2020 at 11:04 AM bensonr ***@***.***> wrote: ***@***.**** requested changes on this pull request. Please see the inlined comments regarding multi-gases namelist reads. ------------------------------ In model/fv_control.F90 <#15 (comment)> : > +#ifdef MULTI_GASES + namelist /multi_gases_nml/ rilist,cpilist +#endif This new version of fv_control has each namelist read contained within its own programmatic unit. Please create a separate subroutine for the multi-gases nml read and add a #ifdef gated call in the proper place. ------------------------------ In model/fv_control.F90 <#15 (comment)> : > @@ -738,26 +1067,12 @@ subroutine run_setup(Atm, dt_atmos, grids_on_this_pe, p_split) read (input_nml_file,multi_gases_nml,iostat=ios) ierr = check_nml_error(ios,'multi_gases_nml') #endif See comment above regarding separate programmatic unit for multi-gases nml. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALRWWEPDW5R6RXTNCCPVQLDRRQB7RANCNFSM4KUZZYTA> .

bensonr · 2020-05-14T16:00:21Z

@HenryJuang-NOAA The comment is not referring to a file unit, but a programmatic unit - i.e. subroutine. Each namelist read in the updated code base is now encapsulated within a separate subroutine. If the multi-gases namelist logic is encapsulated within a separate, dedicated subroutine, it will remove many MULTI_GASES #ifdef-#endif pairs as the subroutine would not need any. In fact, now that I look more closely at the codebase itself, all of the namelist logic regarding multi-gases should be encapsulated within the multi-gases_Init within the multi_gases.F90 source file.

bensonr · 2020-05-14T17:04:35Z

Sure. As mentioned in a later comment, this logic should all be moved to the multi_gases_init routine in multi_gases.F90 and the call to multi_gases_init should be moved to the same level as the calls to the individual namelist reads.

HenryJuang-NOAA · 2020-05-14T17:12:15Z

In fact, I like all nml in one place instead of spreading around different subroutines.... it is easily to find one place for all nml lists instead of all places for one function for nml.... especially, we all are used to the original way, why keep changing.......????? ifdef multi-gases is an option, but it can be in as default, then all ifdef can be removed later... I don't worry too much of ifdef... many options around in the code, I have already used to them... good thing for itdef is that it saves running time if statement and very easy to insert and remove from code...

…

On Thu, May 14, 2020 at 12:00 PM bensonr ***@***.***> wrote: @HenryJuang-NOAA <https://github.com/HenryJuang-NOAA> The comment is not referring to a file unit, but a programmatic unit - i.e. subroutine. Each namelist read in the updated code base is now encapsulated within a separate subroutine. If the multi-gases namelist logic is encapsulated within a separate, dedicated subroutine, it will remove many MULTI_GASES #ifdef-#endif pairs as the subroutine would not need any. In fact, now that I look more closely at the codebase itself, all of the namelist logic regarding multi-gases should be encapsulated within the multi-gases_Init within the multi_gases.F90 source file. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALRWWEI7T3NLN2DSR3JKLNLRRQISNANCNFSM4KUZZYTA> .

HenryJuang-NOAA · 2020-05-14T17:15:48Z

multi-gases originally is for WAM, and you already put it with WAM in RT. If you want to create a new RT for regular gfs in next pull, it is fine to me. It should be better with gfs RT.

…

On Thu, May 14, 2020 at 12:08 PM XiaqiongZhou-NOAA ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In model/fv_control.F90 <#15 (comment)> : > +#ifdef MULTI_GASES + namelist /multi_gases_nml/ rilist,cpilist +#endif Can this be changed in the next pull request when we add the regression test for MULTI_GASES? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALRWWELGB6V74FDDINWWHADRRQJN5ANCNFSM4KUZZYTA> .

To avoid the error for debug

…rt files when write_restart_with_bcs=.true. corresponding to the removal of the halo of delz in new FV3 dycore (by Tom Black)

climbfuji

I did review this PR a few weeks ago, not much has changed since then. I mainly focussed on changes around the CCPP fast physics blocks, looks good to me.

climbfuji · 2020-06-09T13:40:37Z

tools/sorted_index.F90

-!* useful, but WITHOUT ANYWARRANTY; without even the implied warranty 
-!* of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  
+!* The FV3 dynamical core is distributed in the hope that it will be
+!* useful, but WITHOUT ANYWARRANTY; without even the implied warranty


typo: ANYWARRANTY -> ANY WARRANTY

TomBlack-NOAA

Regional module changes in particular appear to be okay.

Merge GFDL new dynamic core 201912 release version to dev/emc.

* Updates needed for fms2_io * Updating variable naming

Dev/emc

Flexible Modeling System and others added 30 commits March 19, 2007 00:00

Import of nalanda.

212a2e8

Import of nalanda_2007_04.

bef6d08

Import of nalanda_2007_06.

e083c51

Import of omsk.

6189161

Import of omsk_2007_10.

f158cd6

Import of omsk_2007_12.

5b7c9e9

Import of omsk_2008_03.

8e27f66

Import of perth.

8efc138

Import of perth_2008_10.

d9e5f4a

Import of quebec.

b618b97

Import of quebec_200910.

3e56e03

Import of riga.

6cfd61e

Import of riga_201004.

1dd5654

Import of riga_201006.

f887872

Import of riga_201012.

fff7ab6

Import of riga_201104.

1c520ad

Import of siena.

811bb0b

Import of siena_201202.

188f27a

Import of siena_201203.

9fa4c1a

Import of siena_201204.

709a586

Import of siena_201207.

3ba00f4

Import of siena_201211.

857b6e7

Import of siena_201303.

a2d182a

Import of siena_201305.

2bf4bfd

Import of siena_201308.

cb3dea7

Import of siena_201309.

3828366

Import of tikal.

cc3804f

Import of tikal_201403.

8384a98

Import of tikal_201409.

8369603

Updated to match CVS testing as of 20141209

e126bfa

XiaqiongZhou-NOAA added 3 commits May 5, 2020 15:55

Fixes needed after merged EMC: dev/emc branch

0fbd13f

Change initial TKE=0 when TKE is absent in external ICs

88b655f

Merge NOAA-EMC dev/emc branch

ee8a899

climbfuji reviewed May 13, 2020

View reviewed changes

TomBlack-NOAA approved these changes May 13, 2020

View reviewed changes

bensonr suggested changes May 14, 2020

View reviewed changes

XiaqiongZhou-NOAA and others added 5 commits May 15, 2020 13:38

Fix an error in fv_regional_bc.F90; Delete unnecessary files

593616a

Update boundary.F90

8ac2d70

To avoid the error for debug

Fixes to write sensible temperature in BCs correctly to the new resta…

34748d9

…rt files when write_restart_with_bcs=.true. corresponding to the removal of the halo of delz in new FV3 dycore (by Tom Black)

Merge remote-tracking branch 'EMC-dev/dev/emc' into dev/emc

06b1f3c

Revise Atm(n)tile to Atm(n)tile_of_mosaic

dbcc271

DusanJovic-NOAA requested review from climbfuji, bensonr, HenryJuang-NOAA and TomBlack-NOAA June 9, 2020 13:26

climbfuji approved these changes Jun 9, 2020

View reviewed changes

TomBlack-NOAA approved these changes Jun 9, 2020

View reviewed changes

HenryJuang-NOAA approved these changes Jun 9, 2020

View reviewed changes

junwang-noaa approved these changes Jun 9, 2020

View reviewed changes

DusanJovic-NOAA merged commit 3ff1ed2 into NOAA-EMC:dev/emc Jun 9, 2020

tsupinie pushed a commit to tsupinie/GFDL_atmos_cubed_sphere that referenced this pull request Jul 20, 2020

Merge GFDL new dynamic core to EMC fork/branch (NOAA-EMC#15)

388c65a

Merge GFDL new dynamic core 201912 release version to dev/emc.

aerorahul pushed a commit that referenced this pull request Jul 17, 2021

Updates needed for fms2_io (#15)

72bbfcc

* Updates needed for fms2_io * Updating variable naming

XiaqiongZhou-NOAA pushed a commit to XiaqiongZhou-NOAA/GFDL_atmos_cubed_sphere that referenced this pull request Sep 22, 2022

Merge pull request NOAA-EMC#15 from NOAA-GFDL/dev/emc

083500c

Dev/emc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge GFDL new dynamic core to EMC fork/branch #15

Merge GFDL new dynamic core to EMC fork/branch #15

XiaqiongZhou-NOAA commented Feb 13, 2020 •

edited

Loading

climbfuji left a comment

climbfuji May 13, 2020

XiaqiongZhou-NOAA May 15, 2020

XiaqiongZhou-NOAA May 15, 2020

climbfuji May 13, 2020

XiaqiongZhou-NOAA May 15, 2020

climbfuji May 13, 2020

climbfuji May 13, 2020

XiaqiongZhou-NOAA May 14, 2020

climbfuji May 13, 2020

bensonr left a comment

bensonr May 14, 2020

XiaqiongZhou-NOAA May 14, 2020

bensonr May 14, 2020

HenryJuang-NOAA commented May 14, 2020 via email

HenryJuang-NOAA commented May 14, 2020 via email

bensonr commented May 14, 2020

bensonr commented May 14, 2020 via email

HenryJuang-NOAA commented May 14, 2020 via email

HenryJuang-NOAA commented May 14, 2020 via email

climbfuji left a comment

climbfuji Jun 9, 2020

TomBlack-NOAA left a comment

Merge GFDL new dynamic core to EMC fork/branch #15

Merge GFDL new dynamic core to EMC fork/branch #15

Conversation

XiaqiongZhou-NOAA commented Feb 13, 2020 • edited Loading

climbfuji left a 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

bensonr left a 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

HenryJuang-NOAA commented May 14, 2020 via email

HenryJuang-NOAA commented May 14, 2020 via email

bensonr commented May 14, 2020

bensonr commented May 14, 2020 via email

HenryJuang-NOAA commented May 14, 2020 via email

HenryJuang-NOAA commented May 14, 2020 via email

climbfuji left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomBlack-NOAA left a comment

Choose a reason for hiding this comment

XiaqiongZhou-NOAA commented Feb 13, 2020 •

edited

Loading