NOAA-GFDL / GFDL_atmos_cubed_sphere

The GFDL atmos_cubed_sphere dynamical core code
Other
56 stars 118 forks source link

202305 Release of GFDL_atmos_cubed_sphere #273

Closed laurenchilutti closed 1 year ago

laurenchilutti commented 1 year ago

Description

This PR publishes GFDL_atmos_cubed_sphere 202305 release. This release coincides with the release of SHiELD_physics 202305.

The changes included in this PR are from the GFDL FV3 Team. Full description of changes can be seen in the RELEASE.md

PRs that are all related and should be merged with this one: https://github.com/NOAA-GFDL/SHiELD_physics/pull/22 https://github.com/NOAA-GFDL/SHiELD_build/pull/22 https://github.com/NOAA-GFDL/atmos_drivers/pull/22

Fixes # (issue)

How Has This Been Tested?

Tested with the regression tests in SHiELD_build

Checklist:

Please check all whether they apply or not

laurenchilutti commented 1 year ago

The CI is expected to fail until I merge in https://github.com/NOAA-GFDL/SHiELD_build/pull/22 because idealized test cases now require a new namelist flag

lharris4 commented 1 year ago

Hi, Rusty. These are all excellent ideas. I am hoping we can work on them soon.

Lucas

On Fri, Jun 2, 2023 at 2:24 PM Rusty Benson @.***> wrote:

@.**** approved this pull request.

Nothing I've noted should stop this PR from being merged, but we should address the issues noted sooner rather than later.

In model/dyn_core.F90 https://github.com/NOAA-GFDL/GFDL_atmos_cubed_sphere/pull/273#discussion_r1214600987 :

@@ -57,6 +58,8 @@ module dyn_core_mod

ifdef SW_DYNAMICS

use test_cases_mod, only: test_case, case9_forcing1, case9_forcing2

endif

  • use test_cases_mod, only: w_forcing

I'm not a big fan of using the parameter from test_cases_mod. If it is something needed by the simulations, it might be better included in flagstruct and defined in fv_control/fv_arrays.

In model/fv_arrays.F90 https://github.com/NOAA-GFDL/GFDL_atmos_cubed_sphere/pull/273#discussion_r1214604432 :

@@ -895,6 +905,8 @@ module fv_arrays_mod real(kind=R_GRID) :: deglat=15. !< Latitude (in degrees) used to compute the uniform f-plane !< Coriolis parameter for doubly-periodic simulations !< (grid_type = 4). The default value is 15.

  • real(kind=R_GRID) :: domain_deg = 0.

We should get a description for this variable.

In model/fv_grid_utils.F90 https://github.com/NOAA-GFDL/GFDL_atmos_cubed_sphere/pull/273#discussion_r1214658426 :

@@ -194,7 +194,7 @@ subroutine grid_utils_init(Atm, npx, npy, npz, non_ortho, grid_type, c2l_order) if (.not. Atm%flagstruct%external_eta) then call set_eta(npz, Atm%ks, Atm%ptop, Atm%ak, Atm%bk, Atm%flagstruct%npz_type, Atm%flagstruct%fv_eta_file) if ( is_master() ) then

  • write(,) 'Grid_init', npz, Atm%ks, Atm%ptop
  • !write(,) 'Grid_init', npz, Atm%ks, Atm%ptop

Should slate this for removal.

In tools/fv_grid_tools.F90 https://github.com/NOAA-GFDL/GFDL_atmos_cubed_sphere/pull/273#discussion_r1214671354 :

@@ -586,7 +585,7 @@ subroutine init_grid(Atm, grid_name, grid_file, npx, npy, npz, ndims, nregions, if (Atm%flagstruct%grid_type>3) then if (Atm%flagstruct%grid_type == 4) then call setup_cartesian(npx, npy, Atm%flagstruct%dx_const, Atm%flagstruct%dy_const, &

  • Atm%flagstruct%deglat, Atm%bd)
  • Atm%flagstruct%deglat, Atm%flagstruct%domain_deg, Atm%bd, Atm)

Because this is sending in elements of the Atm structure and the whole Atm, the interface should be simplified and remove the individual Atm elements as unexpected things can happen with scope INOUT variables.

In tools/fv_restart.F90 https://github.com/NOAA-GFDL/GFDL_atmos_cubed_sphere/pull/273#discussion_r1214677775 :

  • if (gid == sending_proc) then !crazy logic but what we have for now
  • do p=1,size(Atm(1)%pelist)
  • call mpp_send(g_dat,size(g_dat),Atm(1)%pelist(p))
  • enddo endif
  • endif
  • if (ANY(Atm(1)%pelist == gid)) then
  • call mpp_recv(g_dat, size(g_dat), sending_proc)
  • endif

Should be able to use mpp_broadcast with the pelist

In tools/fv_restart.F90 https://github.com/NOAA-GFDL/GFDL_atmos_cubed_sphere/pull/273#discussion_r1214678006 :

  • if (gid == sending_proc) then
  • do p=1,size(Atm(1)%pelist)
  • call mpp_send(g_dat,size(g_dat),Atm(1)%pelist(p))
  • enddo endif
  • endif
  • if (ANY(Atm(1)%pelist == gid)) then
  • call mpp_recv(g_dat, size(g_dat), sending_proc)
  • endif

Should be able to use mpp_broadcast with the given pelist

In tools/fv_restart.F90 https://github.com/NOAA-GFDL/GFDL_atmos_cubed_sphere/pull/273#discussion_r1214678806 :

  • if (gid == sending_proc) then
  • do p=1,size(Atm(1)%pelist)
  • call mpp_send(g_dat,size(g_dat),Atm(1)%pelist(p))
  • enddo endif
  • endif
  • if (ANY(Atm(1)%pelist == gid)) then
  • call mpp_recv(g_dat, size(g_dat), sending_proc)
  • endif

Should be able to use mpp_broadcast here with the given pelist

In tools/fv_restart.F90 https://github.com/NOAA-GFDL/GFDL_atmos_cubed_sphere/pull/273#discussion_r1214679980 :

  • if (gid == sending_proc) then
  • do p=1,size(Atm(1)%pelist)
  • call mpp_send(g_dat,size(g_dat),Atm(1)%pelist(p))
  • enddo endif
  • endif
  • if (ANY(Atm(1)%pelist == gid)) then
  • call mpp_recv(g_dat, size(g_dat), sending_proc)
  • endif

Should be able to use mpp_broadcast here with the given pelist

In tools/fv_restart.F90 https://github.com/NOAA-GFDL/GFDL_atmos_cubed_sphere/pull/273#discussion_r1214680082 :

  • if (gid == sending_proc) then
  • do p=1,size(Atm(1)%pelist)
  • call mpp_send(g_dat,size(g_dat),Atm(1)%pelist(p))
  • enddo endif
  • endif
  • if (ANY(Atm(1)%pelist == gid)) then
  • call mpp_recv(g_dat, size(g_dat), sending_proc)
  • endif

Should be able to use mpp_broadcast here with the given pelist

In tools/fv_restart.F90 https://github.com/NOAA-GFDL/GFDL_atmos_cubed_sphere/pull/273#discussion_r1214680186 :

  • if (gid == sending_proc) then
  • do p=1,size(Atm(1)%pelist)
  • call mpp_send(g_dat,size(g_dat),Atm(1)%pelist(p))
  • enddo endif
  • endif
  • if (ANY(Atm(1)%pelist == gid)) then
  • call mpp_recv(g_dat, size(g_dat), sending_proc)
  • endif

Should be able to use mpp_broadcast here with the given pelist

In tools/fv_restart.F90 https://github.com/NOAA-GFDL/GFDL_atmos_cubed_sphere/pull/273#discussion_r1214680264 :

  • if (gid == sending_proc) then
  • do p=1,size(Atm(1)%pelist)
  • call mpp_send(g_dat,size(g_dat),Atm(1)%pelist(p))
  • enddo endif
  • endif
  • if (ANY(Atm(1)%pelist == gid)) then
  • call mpp_recv(g_dat, size(g_dat), sending_proc)
  • endif

Should be able to use mpp_broadcast here with the given pelist

In tools/fv_restart.F90 https://github.com/NOAA-GFDL/GFDL_atmos_cubed_sphere/pull/273#discussion_r1214680391 :

  • if (gid == sending_proc) then
  • do p=1,size(Atm(1)%pelist)
  • call mpp_send(g_dat,size(g_dat),Atm(1)%pelist(p))
  • enddo endif
  • endif
  • if (ANY(Atm(1)%pelist == gid)) then
  • call mpp_recv(g_dat, size(g_dat), sending_proc)
  • endif

Should be able to use mpp_broadcast here with the given pelist

— Reply to this email directly, view it on GitHub https://github.com/NOAA-GFDL/GFDL_atmos_cubed_sphere/pull/273#pullrequestreview-1458013688, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMUQRVFVGUDGDXFLCKKVSA3XJIVVDANCNFSM6AAAAAAYW4UKLQ . You are receiving this because your review was requested.Message ID: @.***>