Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cam6_3_016: Refactor coupling with WACCMX ionosphere #264

Merged
merged 40 commits into from Apr 1, 2021

Conversation

fvitt
Copy link

@fvitt fvitt commented Nov 2, 2020

This PR is a refactor of how WACCMX ionosphere is coupled to CAM to be independent of dynamical core.

Previously, the FV dycore grid was used as the ion transport grid. Thus, WACCMX could only be run with FV dycore.

Here, ESMF regridding utilities are used to regrid fields between independent ion transport lat/lon grid, electro-dynamo geomagnetic lat/lon grid, and a generalized cam physics grid mesh. The use of an ESMF gridded component to contain the ion transport and electro-dynamo ionosphere components allows the ionosphere to be executed on a subset of CAM's MPI tasks and for multi-instance WACCMX configurations (a requirement for WACCMX-DART).

closes #84
closes #223
closes #151

        deleted:    cime_config/testdefs/testmods_dirs/cam/outfrq3s_WX81/shell_commands
        deleted:    cime_config/testdefs/testmods_dirs/cam/outfrq3s_WX81/user_nl_cam
        deleted:    cime_config/testdefs/testmods_dirs/cam/outfrq3s_WX81/user_nl_clm
        modified:   cime_config/testdefs/testmods_dirs/cam/outfrq9s_576tsks/user_nl_clm
        modified:   cime_config/config_pes.xml
        modified:   cime_config/testdefs/testlist_cam.xml
        modified:   src/dynamics/se/dp_coupling.F90
        modified:   src/dynamics/se/dycore/dimensions_mod.F90
        modified:   src/dynamics/se/dycore/global_norms_mod.F90
        modified:   src/dynamics/se/dycore/prim_advance_mod.F90
        modified:   src/dynamics/se/dyn_comp.F90
        modified:   src/dynamics/se/stepon.F90
…ning of each time step; misc cleanup

        deleted:    src/unit_drivers/wxie/unit_driver.F90
        modified:   bld/config_files/definition.xml
        modified:   bld/configure
        modified:   cime_config/testdefs/testlist_cam.xml
        modified:   src/physics/cam/physpkg.F90
        modified:   src/physics/cam/rk_stratiform.F90
        modified:   src/physics/cam/waccmx_phys_intr.F90
        modified:   src/physics/waccmx/ion_electron_temp.F90
        modified:   src/unit_drivers/drv_input_data.F90
        modified:   src/chemistry/mozart/mo_apex.F90
        modified:   src/utils/physconst.F90
        modified:   bld/namelist_files/namelist_defaults_cam.xml
        modified:   cime_config/testdefs/testlist_cam.xml
        modified:   bld/namelist_files/namelist_defaults_cam.xml
        modified:   bld/build-namelist
        modified:   bld/namelist_files/namelist_defaults_cam.xml
        modified:   cime_config/testdefs/testlist_cam.xml
        modified:   doc/ChangeLog
        modified:   src/ionosphere/waccmx/ionosphere_interface.F90
        modified:   cime_config/testdefs/testlist_cam.xml
        deleted:    cime_config/testdefs/testmods_dirs/cam/outfrq3s_waccm_ma/shell_commands
        deleted:    cime_config/testdefs/testmods_dirs/cam/outfrq3s_waccm_ma/user_nl_cam
        new file:   cime_config/testdefs/testmods_dirs/cam/outfrq9s_multi/shell_commands
        new file:   cime_config/testdefs/testmods_dirs/cam/outfrq9s_multi/user_nl_cam
        renamed:    cime_config/testdefs/testmods_dirs/cam/outfrq3s_waccm_ma/user_nl_clm -> cime_config/testdefs/testmods_dirs/cam/outfrq9s_multi/user_nl_clm
@fvitt fvitt added enhancement New feature or request answer changing answer changing tag labels Nov 2, 2020
@fvitt fvitt added this to the CESM2.3 milestone Nov 2, 2020
@fvitt fvitt self-assigned this Nov 2, 2020
@fvitt fvitt added this to Upcoming tags - in review and testing in CAM Development branch (cutting edge development) via automation Nov 2, 2020
        modified:   src/ionosphere/waccmx/edyn_geogrid.F90
        modified:   src/ionosphere/waccmx/edyn_grid_comp.F90
        modified:   src/ionosphere/waccmx/regridder.F90
Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks ok to me! I just have some clean-up and clarification requests.

Also, one other issue I have that is not listed below is that in some of the source files the WACCMX_EDYN_ESMF def is so prevalent that it impacts readability (at least for me). I assume this pre-proc def is necessary in order to prevent users from having to build ESMF?

If so then it might be good to create a Github issue stating that this WACCMX_EDYN_ESMF def can be replaced with a fortran run-time option once NUOPC/ESMF becomes the default CAM configuration (which I can do, I just want to make sure I am understanding the purpose of WACCMX_EDYN_ESMF correctly). Thus any info you can provide would certainly be appreciated.

bld/configure Outdated Show resolved Hide resolved
bld/namelist_files/use_cases/waccmx_ma_2000_cam4.xml Outdated Show resolved Hide resolved
doc/ChangeLog Show resolved Hide resolved
src/ionosphere/waccmx/adotv_mod.F90 Outdated Show resolved Hide resolved
src/ionosphere/waccmx/adotv_mod.F90 Outdated Show resolved Hide resolved
src/ionosphere/waccmx/edyn_esmf.F90 Outdated Show resolved Hide resolved
src/ionosphere/waccmx/edyn_esmf.F90 Outdated Show resolved Hide resolved
src/ionosphere/waccmx/edyn_esmf.F90 Outdated Show resolved Hide resolved
src/ionosphere/waccmx/edyn_esmf.F90 Outdated Show resolved Hide resolved
src/ionosphere/waccmx/edyn_esmf.F90 Outdated Show resolved Hide resolved
@fvitt
Copy link
Author

fvitt commented Dec 30, 2020

@nusbaume,
The WACCMX_EDYN_ESMF cpp variable is intended to allow ion transport without the need for the ESMF library and without an active electro-dynamo. This was the "-ionosphere wxi" configure option which was unsupported and was removed in this PR. We may be able to remove the WACCMX_EDYN_ESMF cpp defs from the code. I will explore this possibility.

        modified:   bld/namelist_files/namelist_defaults_cam.xml
        modified:   cime_config/testdefs/testmods_dirs/cam/outfrq9s_576tsks/shell_commands
        modified:   src/ionosphere/waccmx/edyn_grid_comp.F90
        modified:   bld/configure
        modified:   src/ionosphere/waccmx/amie_module.F90
        modified:   src/ionosphere/waccmx/edyn_esmf.F90
        modified:   src/ionosphere/waccmx/edyn_init.F90
        modified:   src/ionosphere/waccmx/edynamo.F90
        modified:   src/ionosphere/waccmx/ionosphere_interface.F90
        modified:   src/ionosphere/waccmx/ltr_module.F90
        modified:   src/ionosphere/waccmx/utils_mod.F90
        modified:   bld/configure
        modified:   src/ionosphere/waccmx/adotv_mod.F90
        modified:   src/ionosphere/waccmx/amie_module.F90
        modified:   src/ionosphere/waccmx/dpie_coupling.F90
        modified:   src/ionosphere/waccmx/edyn_init.F90
        modified:   src/ionosphere/waccmx/edyn_params.F90
        modified:   src/ionosphere/waccmx/ionosphere_interface.F90
        modified:   src/ionosphere/waccmx/ltr_module.F90
        modified:   src/ionosphere/waccmx/utils_mod.F90
        modified:   src/physics/cam/cam_diagnostics.F90
        modified:   src/physics/cam/phys_control.F90
   These are reverted to earlier version:
        modified:   bld/namelist_files/namelist_defaults_cam.xml
        modified:   src/ionosphere/waccmx/edyn_grid_comp.F90
        modified:   src/ionosphere/waccmx/edyn_grid_comp.F90
        modified:   src/ionosphere/waccmx/edyn_init.F90
@fvitt fvitt requested review from nusbaume and gold2718 March 3, 2021 15:47
Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me now. Thanks for getting these major modifications ready!

@nusbaume nusbaume marked this pull request as ready for review March 4, 2021 15:59
bld/namelist_files/namelist_defaults_cam.xml Show resolved Hide resolved
cime_config/testdefs/testlist_cam.xml Outdated Show resolved Hide resolved
cime_config/testdefs/testlist_cam.xml Outdated Show resolved Hide resolved
@@ -25,3 +25,4 @@ hist_nhtfrq = 9
hist_mfilt = 1
hist_ndens = 1

fsurdat = '/glade/p/cesmdata/cseg/inputdata/lnd/clm2/surfdata_map/surfdata_0.47x0.63_78pfts_CMIP6_simyr2000_c180508.nc'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I verified this file exists in the svn repo, so it should be using a relative path in case the test is used anywhere other than cheyenne in the future.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file path is now relative to $DIN_LOC_ROOT

src/ionosphere/waccmx/edyn_geogrid.F90 Outdated Show resolved Hide resolved
src/ionosphere/waccmx/edyn_grid_comp.F90 Outdated Show resolved Hide resolved
bld/configure Outdated Show resolved Hide resolved
src/ionosphere/waccmx/wei05sc.F90 Outdated Show resolved Hide resolved
…r f19 res; correct path for fsurdat file in test

        modified:   bld/namelist_files/namelist_defaults_cam.xml
        modified:   cime_config/config_pes.xml
        modified:   cime_config/testdefs/testmods_dirs/cam/outfrq9s_576tsks/user_nl_clm
        modified:   src/ionosphere/waccmx/edyn_geogrid.F90
        modified:   src/ionosphere/waccmx/edyn_grid_comp.F90
        modified:   src/ionosphere/waccmx/oplus.F90
        modified:   src/ionosphere/waccmx/wei05sc.F90
        modified:   src/ionosphere/waccmx/edyn_mpi.F90
        modified:   bld/build-namelist
        modified:   bld/config_files/definition.xml
        modified:   bld/configure
        modified:   bld/namelist_files/namelist_defaults_cam.xml
        modified:   bld/namelist_files/namelist_definition.xml
        modified:   src/chemistry/mozart/short_lived_species.F90
        modified:   src/ionosphere/waccmx/edyn_init.F90
        modified:   src/ionosphere/waccmx/edyn_maggrid.F90
        modified:   src/ionosphere/waccmx/edyn_mud.F90
        modified:   src/ionosphere/waccmx/edyn_mudmod.F90
        modified:   src/ionosphere/waccmx/edyn_muh2cr.F90
        modified:   src/ionosphere/waccmx/edyn_solve.F90
        modified:   src/ionosphere/waccmx/edynamo.F90
        modified:   src/ionosphere/waccmx/getapex.F90
        modified:   src/ionosphere/waccmx/ionosphere_interface.F90
        modified:   src/ionosphere/waccmx/wei05sc.F90
Copy link
Collaborator

@gold2718 gold2718 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a couple of questions and a few suggestions but I think it is basically ready to go. It still needs some work to get up to standard but it is a whole lot better.

Comment on lines 68 to 69
nlon_geo, & ! size of geo lon dimension
nlat_geo, & ! size of geo lat dimension
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two variables are private so the protected attribute has no effect. Are they supposed to be public? The same question goes for some of the other variables below.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the protected attribute from the private variables in this module.

Comment on lines 249 to 251
write(iulog,"('mp_distribute_geo: mytid=',i4,' ntaski,j=',2i4,' mytidi,j=',2i4,&
' lon0,1=',2i4,' lat0,1=',2i4,' lev0,1=',2i4)") &
mytid,ntaski,ntaskj,mytidi,mytidj,lon0,lon1,lat0,lat1,lev0,lev1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like these write statements (here and below) are intended to come from each active task. For those, I suggest writing to unit 6 so that all tasks write to the same file.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now writing to unit 6.

Comment on lines 272 to 274
! Report my stats to stdout:
! write(iulog,"(/,'mytid=',i3,' mytidi,j=',2i3,' lat0,1=',2i3,' (',i2,') lon0,1=',2i3,' (',i2,') ncells=',i4)") &
! mytid,mytidi,mytidj,lat0,lat1,nj,lon0,lon1,ni
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest either deleting this comment or un-commenting and wrapping in debug.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put the write statement inside a if debug block.

Comment on lines 411 to 414
write(iulog,"(/,a,i3,a,2i3,a,2i3,a,i2,2a,2i3,a,i2,a,i4)") &
'mytid = ',mytid, ', magtidi,j = ', magtidi, magtidj, &
', mlat0,1 = ', mlat0, mlat1, ' (', nj, ')', &
', mlon0,1 = ', mlon0, mlon1, ' (', ni, ') ncells = ', ncells
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another place where writing to 6 might help.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now writing to unit 6.

@@ -1,26 +1,24 @@
!-----------------------------------------------------------------------
subroutine mudmod(pe,phi_out,jntl,isolve,ier)
subroutine mudmod(pe,phi_out,jntl,isolve,nlev,ier)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not a module?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now inside a module.

@@ -867,10 +966,10 @@ subroutine solver(cofum,c0)
jntl = 0
ier = 0
isolve = 2
call mudmod(rim_glb,phisolv,jntl,isolve,ier)! solver in mudmod.F
call mudmod(rim_glb,phisolv,jntl,isolve,res_nlev,ier)! solver in mudmod.F
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is incorrect. If edyn_mudmod.F90 became a module, it would be clear where this came from.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the comment.

@@ -2,21 +2,24 @@
subroutine mud(pe,jntl,isolve)
use shr_kind_mod ,only: r8 => shr_kind_r8
use cam_abortutils ,only: endrun
use edyn_solve,only: nc,ncee,cee
use edyn_solve,only: nc,cee
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not a module? Is this routine (mud) still used anywhere?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The subroutine mud was not used. I removed it. The remaining routines are inside a module.

public :: edyn_grid_comp_run2
public :: edyn_grid_comp_final

! Private data and interfaces
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no private statement so aren't all these symbols really public? Maybe put a private statement in.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a private statement.

call ESMF_GridCompSetServices(phys_comp, edyn_gcomp_SetServices, rc=rc)
call edyn_esmf_chkerr(subname, 'ESMF_GridCompSetServices '//trim(inst_name), rc)
! Initialize the required component arguments
call ESMF_GridCompInitialize(phys_comp, rc=rc)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you tell me why you did not need import and export states? It is much simpler! (also for run and finalize)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import and export states are optional arguments. It works as it is. If you would like a better answer, I will need some time to study the code and think about it.

@@ -1,3 +1,166 @@

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you finish this file, please be sure to separate the 'A' (adds) and 'D' (deletes) into their correct locations.

fvitt and others added 5 commits March 27, 2021 13:31
        modified:   src/chemistry/mozart/short_lived_species.F90
        modified:   src/ionosphere/waccmx/edyn_geogrid.F90
        modified:   src/ionosphere/waccmx/edyn_init.F90
        modified:   src/ionosphere/waccmx/edyn_mud.F90
        modified:   src/ionosphere/waccmx/edyn_mudmod.F90
        modified:   src/ionosphere/waccmx/edyn_muh2cr.F90
        modified:   src/ionosphere/waccmx/edyn_solve.F90
        modified:   src/ionosphere/waccmx/getapex.F90
        modified:   src/ionosphere/waccmx/ionosphere_interface.F90
        modified:   src/ionosphere/waccmx/edyn_grid_comp.F90
        modified:   src/ionosphere/waccmx/edyn_mpi.F90
        modified:   src/ionosphere/waccmx/edyn_mud.F90
        modified:   src/ionosphere/waccmx/edyn_mudcom.F90
        modified:   src/ionosphere/waccmx/edyn_mudmod.F90
        modified:   src/ionosphere/waccmx/edyn_muh2cr.F90
        modified:   src/ionosphere/waccmx/edyn_solve.F90
        new file:   src/ionosphere/waccmx/edyn_solver_coefs.F90
        modified:   src/ionosphere/waccmx/edyn_mudcom.F90
@cacraigucar cacraigucar moved this from Upcoming tags - in review and testing to Tag frozen/regression testing in CAM Development branch (cutting edge development) Mar 30, 2021
@cacraigucar cacraigucar changed the title Refactor coupling with WACCMX ionosphere cam6_3_016: Refactor coupling with WACCMX ionosphere Mar 30, 2021
@fvitt fvitt merged commit 58e9d7c into ESCOMP:cam_development Apr 1, 2021
CAM Development branch (cutting edge development) automation moved this from Tag frozen/regression testing to Completed tags Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
answer changing answer changing tag enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants