Skip to content

Remove nems grid comp#104

Merged
MinsukJi-NOAA merged 4 commits into
NOAA-EMC:developfrom
DusanJovic-NOAA:remove_nems_grid_comp
Jul 16, 2021
Merged

Remove nems grid comp#104
MinsukJi-NOAA merged 4 commits into
NOAA-EMC:developfrom
DusanJovic-NOAA:remove_nems_grid_comp

Conversation

@DusanJovic-NOAA
Copy link
Copy Markdown
Collaborator

@DusanJovic-NOAA DusanJovic-NOAA commented Jun 21, 2021

  • Remove NEMS grid component. The Earth grid component is now instantiated in the main program.
  • Remove import/export stated from the main program.
  • Code cleanup.

Fixes: #103

@DeniseWorthen
Copy link
Copy Markdown
Contributor

Would it be possible to remove the ALLCAPS and just use lower case where possible? (eg INTEGER,INTENT(OUT) :: RC_REG)

@DusanJovic-NOAA
Copy link
Copy Markdown
Collaborator Author

DusanJovic-NOAA commented Jun 21, 2021

Would it be possible to remove the ALLCAPS and just use lower case where possible? (eg INTEGER,INTENT(OUT) :: RC_REG)

Do you mean in module_EARTH_GRID_COMP.F90?

BTW, I think this file (module) should be renamed to ufs_nuopc_driver.F90 because that's what it is.

Comment thread src/MAIN_NEMS.F90
!-----------------------------------------------------------------------
!
contains
subroutine check_esmf_pet(print_esmf)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

model_configure is an ESMF config file. It should be opened and queried with:

ESMF_ConfigLoadFile
ESMF_ConfigGetAttribute

see: http://earthsystemmodeling.org/docs/release/ESMF_8_1_1/ESMF_refdoc/node6.html#SECTION06093900000000000000

That being said, I do not think that NEMS should open "model_configure" at all, since that it an atmosphere configuration file. Configuration related to NEMS should go into nems.configure.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is related to my question about clocks. Do we need main clock to be set in the main program. If we do then we must parse all clock related parameters from "model_configure" in the main program. @theurich

This subroutine check_esmf_pet is called before ESMF is initialized to set logging option at the time we call ESMF_Initialize. I'll be happy to remove this subroutine if we can set the ESMF logging after it has been initialized. Can we?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The decision about ESMF logging options to be used definitely does NOT belong into the ATM specific model_configure. Instead it belongs into the nems.configure.

Now how to read this, there is currently this chicken-and-egg issue going on. ESMF_Initialize() needs to be called before you can use ESMF_Config* calls to open the configuration file to read, e.g. to find out what logging option to be used. While you can change some logging details after ESMF_Initialize(), the LOGKIND (e.g. ESMF_LOGKIND_MULTI_ON_ERROR vs ESMF_LOGKIND_MULTI) has to be set during ESMF_Initialize().

CESM is dealing with I think by using a standard Fortran namelist file where they read in information before ESMF_Initialize() is called.

We have been discussing a few other options in the past, but none are implemented. One option I could see is to define a specific Config Attribute, e.g. ESMF_LogKind_Flag. Then have it where when ESMF_Initialize() is called with a specific config file (i.e. via the existing argument 'defaultConfigFileName'), and that config file contains the ESMF_LogKind_Flag attribute, its value will be used during ESMF_Initialize(). For NEMS I would assume that nems.configure was passed in for defaultConfigFileName, and then this provides the mechanism to cleanly set ESMF_LogKind_Flag for the execution.

If we go this direction, we need to prioritize the ESMF work for it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@DusanJovic-NOAA Is this resolved?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not in this PR. We need to keep check_esmf_pet for now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@DusanJovic-NOAA - I know this PR is merged and closed. But there is a new feature in ESMF that directly relates to the discussion about the need for subroutine check_esmf_pet().

Starting with https://github.com/esmf-org/esmf/releases/tag/ESMF_8_2_0_beta_snapshot_18, the 'defaultConfigFileName' argument in ESMF_Initialize() is implemented! For the associated API doc please see: https://earthsystemmodeling.org/docs/nightly/develop/ESMF_refdoc/node4.html#SECTION04024100000000000000

Bottom line, I hope that we can migrate where

  1. ESMF_Initialize(defaultConfigFileName="nems.configure", ...) is called in MAIN_NEMS.F90.
  2. logKindFlag: is set in file nems.configure.
  3. The print_esmf flag is removed from model_configure.
  4. subroutine check_esmf_pet() is removed from MAIN_NEMS.F90, and the logic around calling ESMF_Initialize() is simplified.

Let me know if you have questions about any of what I am proposing.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We are in the process of updating UFS to ESMF_820bs14. I'm not sure when we are going to move to bs18 or newer, but as soon as we do, I'll make these changes and test them. I agree we need to simplify this part of the code and stop using this hand written check_esmf_pet routine.

@DeniseWorthen
Copy link
Copy Markdown
Contributor

DeniseWorthen commented Jun 21, 2021

Would it be possible to remove the ALLCAPS and just use lower case where possible? (eg INTEGER,INTENT(OUT) :: RC_REG)

Do you mean in module_EARTH_GRID_COMP.F90?

BTW, I think this file (module) should be renamed to ufs_nuopc_driver.F90 because that's what it is.

Yes, in the earth grid, I was referring to the code itself where it is written in all caps, for example

      PRIVATE
!
      PUBLIC :: EARTH_REGISTER
      PUBLIC :: VERBOSE_DIAGNOSTICS

@MinsukJi-NOAA
Copy link
Copy Markdown
Collaborator

@junwang-noaa @DeniseWorthen please review and approve

@MinsukJi-NOAA MinsukJi-NOAA merged commit 4fc6d1e into NOAA-EMC:develop Jul 16, 2021
@DusanJovic-NOAA DusanJovic-NOAA deleted the remove_nems_grid_comp branch July 16, 2021 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove PE_MEMBER logic that is no longer relevant

6 participants