[lizziel]

This PR contains fixes for two issues in AdvCore when using MAPL3:

1. Bug getting number of vertical levels during SetServices (Bug getting number of vertical levels from AdvCore SetServices #420). In AdvCore SetServices, num_levels is now set by reading hconfig rather than retrieval from MAPL, where it is not yet set.
2. Requirement for geometry section in both top-level and mapl sections of hconfig. Previously when doing advcore-only runs (no dyncore) MAPL gets the grid information from geometry in the mapl section of advcore.yaml, while FV_StateMod.F90 subroutine get_imi_work_and_topology as well as fv_setup get grid information from the geometry in the top-level section of the same file. This required duplicate sections in the same file. This PR updates that to first look in the mapl section, and then look into the top-level section only if it is not found.

A few unused local variables are also removed in this PR.

[mathomp4]

I've added @pchakraborty as a reviewer since he knows the code better than any of us!

[pchakraborty]

@lizziel Would it work if all the changes are moved to AdvCore instead? Both problems can be fixed entirely in AdvCore_GridCompMod.F90.

For num_levels, fv_setup could move from SetServices to Initialize, which is where DynCore calls FV_Setup. By then MAPL has the vertical grid, so MAPL_GridCompGet(num_levels=) returns the right value and we don't need to read vertical_grid from the config. Nothing else in AdvCore's SetServices uses FV_Atm.

       ! pchakrab - maybe use the FV_DynCoreIsRunning flag instead
       if (.not. FV3_DynCoreIsRunning) then
          call MAPL_GridCompSetGeometry(gc, _RC)
+         ! Done here, not in SetServices: num_levels is not known until Initialize
+         call fv_setup(gc, _RC)
       end if

For the duplicate geometry, AdvCore can pass its mapl: section to setup_fv_dimensions_and_topology when the geometry is there, and the full config otherwise. get_im_world_and_topology then finds geometry:esmf_geom without any change.

The updated fv_setup, called from Initialize instead of SetServices:

    subroutine fv_setup(gc, rc)
       type(ESMF_GridComp), intent(inout) :: gc
       integer, intent(out) :: rc

       type(ESMF_VM) :: vm
-      type(ESMF_HConfig) :: hconfig
+      type(ESMF_HConfig) :: hconfig, geom_parent_cfg
       integer :: p_split = 1
-      integer :: comm, im_world, topology(2), num_levels, status
+      integer :: comm, num_levels, status
+      logical :: has_mapl_section, use_mapl_section

       call ESMF_VMGetCurrent(vm, _RC)
       call ESMF_VMGet(vm, mpiCommunicator=comm, _RC)
       call fms_init(comm)

       call fv_init1(FV_Atm, dt, grids_on_my_pe, p_split)

       call MAPL_GridCompGet(gc, hconfig=hconfig, num_levels=num_levels, _RC)
-      call setup_fv_dimensions_and_topology(hconfig, num_levels, FV_Atm(1)%flagstruct, FV_Atm(1)%layout, _RC)
+
+      ! setup_fv_dimensions_and_topology reads geometry:esmf_geom from the
+      ! config it is given; pass the mapl section if the geometry lives there
+      use_mapl_section = .false.
+      has_mapl_section = ESMF_HConfigIsDefined(hconfig, keyString="mapl", _RC)
+      if (has_mapl_section) then
+         geom_parent_cfg = ESMF_HConfigCreateAt(hconfig, keyString="mapl", _RC)
+         use_mapl_section = ESMF_HConfigIsDefined(geom_parent_cfg, keyString="geometry", _RC)
+         if (.not. use_mapl_section) call ESMF_HConfigDestroy(geom_parent_cfg, _RC)
+      end if
+      if (.not. use_mapl_section) geom_parent_cfg = hconfig
+
+      call setup_fv_dimensions_and_topology(geom_parent_cfg, num_levels, FV_Atm(1)%flagstruct, FV_Atm(1)%layout, _RC)
+      if (use_mapl_section) call ESMF_HConfigDestroy(geom_parent_cfg, _RC)
+
       call fv_init2(FV_Atm, dt, grids_on_my_pe, p_split)

       _RETURN(_SUCCESS)
    end subroutine fv_setup

I'd like to keep changes to FV_StateMod.F90 as small as possible. It's shared with DynCore, and we'll need to bring it back in line with develop eventually, so every MAPL3-only change there makes that merge harder.

[lizziel]

For num_levels, fv_setup could move from SetServices to Initialize, which is where DynCore calls FV_Setup. By then MAPL has the vertical grid, so MAPL_GridCompGet(num_levels=) returns the right value and we don't need to read vertical_grid from the config.

I agree that is a better fix. But you opened a PR with that change but then closed it with a note that we would go with my approach instead: #422. Do you remember why?

[pchakraborty]

For num_levels, fv_setup could move from SetServices to Initialize, which is where DynCore calls FV_Setup. By then MAPL has the vertical grid, so MAPL_GridCompGet(num_levels=) returns the right value and we don't need to read vertical_grid from the config.

I agree that is a better fix. But you opened a PR with that change but then closed it with a note that we would go with my approach instead: #422. Do you remember why?

Possibly because there was no activity on the PR, and it was my understanding that you had a workaround.

[lizziel]

Yes, this PR has the work-around. I can remove it if you want your fix instead and limit this PR to the duplicate geometry issue. Your closed PR can be reopened for merge.

[pchakraborty]

#422 is merged.

[lizziel]

All set for another review.

[pchakraborty]

LGTM