search

NOAA-EMC / NEMS

NEMS (NOAA Environmental Modeling System)
https://noaa-emc.github.io/NEMS_doc/index.html
Other
11 stars 46 forks source link

Update NEMS for CDEPS component #92

Closed binli2337 closed 3 years ago

binli2337 commented 3 years ago
  1. The existing data atmosphere model component will be renamed "nems_data".
  2. The CDEPS data atmosphere model component will be added to module_EARTH_GRID_COMP.F90. The name of the CDEPS data atmosphere model component will be "datm".

fixes: #99

junwang-noaa commented 3 years ago

Maybe "datm" is also used as "ATM_model" in nems.configure for cdeps datm, but inside CDEPS it has its own namalist to specify which source to use.

On Thu, Mar 18, 2021 at 12:47 PM Denise Worthen @.***> wrote:

@DeniseWorthen commented on this pull request.

In src/module_EARTH_GRID_COMP.F90 https://github.com/NOAA-EMC/NEMS/pull/92#discussion_r597060968:

@@ -3596,8 +3599,9 @@ subroutine SetModelServices(driver, rc) file=FILE, rcToReturn=rc) return ! bail out

endif

  • elseif (trim(model) == "datm") then -#ifdef FRONT_DATM
  • elseif (trim(model) == "datm" .or. trim(model) == "datm_cfsr" .or. &

@rsdunlapiv https://github.com/rsdunlapiv So you're saying that nems.configure will have the atm model either datm (=nemsdatm), datm_cfsr (=cdeps,cfsr source) or datm_gefs (cdeps,gefs source). There is nothing in the driver it self that is needed to distinguish those options.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/NOAA-EMC/NEMS/pull/92#discussion_r597060968, or unsubscribe https://github.com/notifications/unsubscribe-auth/AI7D6TKNCPFXORDYL2UPKPLTEIVCDANCNFSM4ZA4BGBA .

uturuncoglu commented 3 years ago

I think there is no need to provide datm or datm_cfsr to nems.configure. Driver does not need to know which data stream is used. This is already controlled by data component specific namelist files. So, I am using datm for data atmosphere for all the configurations including NEMS DATM and works smoothly.

On Mar 18, 2021, at 10:47 AM, Denise Worthen @.***> wrote:

@DeniseWorthen commented on this pull request.

In src/module_EARTH_GRID_COMP.F90 https://github.com/NOAA-EMC/NEMS/pull/92#discussion_r597060968:

@@ -3596,8 +3599,9 @@ subroutine SetModelServices(driver, rc) file=FILE, rcToReturn=rc) return ! bail out

endif

  • elseif (trim(model) == "datm") then -#ifdef FRONT_DATM
  • elseif (trim(model) == "datm" .or. trim(model) == "datm_cfsr" .or. & @rsdunlapiv https://github.com/rsdunlapiv So you're saying that nems.configure will have the atm model either datm (=nemsdatm), datm_cfsr (=cdeps,cfsr source) or datm_gefs (cdeps,gefs source). There is nothing in the driver it self that is needed to distinguish those options.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/NOAA-EMC/NEMS/pull/92#discussion_r597060968, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJMBR4TN2ROPEJTH6GYDO3TEIVCDANCNFSM4ZA4BGBA.

rsdunlapiv commented 3 years ago

@junwang-noaa @DeniseWorthen yes, I think @junwang-noaa has this right. The driver just knows that there is a data atmopshere component that must be included and the run sequence says where to call it. At a lower level in the configuration is where DATM is actually configured with some specific input data streams.

DeniseWorthen commented 3 years ago

OK, thanks. I think we're all on the same page here? Do we actually need any changes to module_EARTH_GRID_COMP.F90?

binli2337 commented 3 years ago

The NEMS_datm and CDEPS_DATM (here datm_cfsr or datm_gefs) use different land-sea masks. Once NEMS_datm is removed, this temporary fix can also be removed. Please also see the temporary fix in med_map_mod.F90 in CMEPS.

DeniseWorthen commented 3 years ago

@binli2337 I think I understand your point. But if you used 

if (model(1:4) == "datm") then

Then no change would be required to this routine when we drop the nemsDATM. Is that right?

junwang-noaa commented 3 years ago

Bin, I think the current CMEPS version works for other CDEPS sources, does this mean "datm_cfsr" and "datm_gefs" are different from other CDEPS data sources? And other CDEPS data sources are working the same way as NEMSdatm?

On Thu, Mar 18, 2021 at 2:19 PM Denise Worthen @.***> wrote:

@binli2337 https://github.com/binli2337 I think I understand your point. But if you used

if (model(1:4) == "datm") then

Then no change would be required to this routine when we drop the nemsDATM. Is that right?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NOAA-EMC/NEMS/pull/92#issuecomment-802180898, or unsubscribe https://github.com/notifications/unsubscribe-auth/AI7D6TN4OH7LUNXQ4RPHA4DTEI74TANCNFSM4ZA4BGBA .

binli2337 commented 3 years ago

@junwang-noaa , The current CMEPS version works for other CDEPS sources only under "hafs" coupling mode.

uturuncoglu commented 3 years ago

@binli2337 I think that the masking of the data component can be modified by changing mask variable in ESMF Mesh file but the only important point is that the masking of the components are handled in the CMEPS in the following line https://github.com/ESCOMP/CMEPS/blob/ad106b0208739b819dba98a88225edfd6e093773/mediator/med_map_mod.F90#L279 and currently we set it as 1 for datm (ERA5). So, you might want to be consistent with that. This part of code needs to be more generic and requires work in CMEPS side.

binli2337 commented 3 years ago

@DeniseWorthen @junwang-noaa @rsdunlapiv @uturuncoglu I have revised module_EARTH_GRID_COMP.F90 to separate "nems_datm" and "datm" (used for CDEPS). Please take a look to see if the changes are OK.

In the past, when CICE5 and MOM5 were removed, the parts related to cice5 and mom5 remained in the module_EARTH_GRID.COM.F90 file.

When nems_datm is removed in the future, the nems_datm related parts can also remain in the module_EARTH_GRID_COM.F90 file.

junwang-noaa commented 3 years ago

Bin, I saw there is lots of legacy code in NEMS, I think we do need to clean them up since the components are not available in UFS.

On Mon, Mar 22, 2021 at 9:14 AM BinLi-NOAA @.***> wrote:

@DeniseWorthen https://github.com/DeniseWorthen @junwang-noaa https://github.com/junwang-noaa @rsdunlapiv https://github.com/rsdunlapiv @uturuncoglu https://github.com/uturuncoglu I have revised module_EARTH_GRID_COMP.F90 to separate "nems_datm" and "datm" (used for CDEPS). Please take a look to see if the changes are OK.

In the past, when CICE5 and MOM5 were removed, the parts related to cice5 and mom5 remained in the module_EARTH_GRID.COM.F90 file.

When nems_datm is removed in the future, the nems_datm related parts can also remain in the module_EARTH_GRID_COM.F90 file.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NOAA-EMC/NEMS/pull/92#issuecomment-804050890, or unsubscribe https://github.com/notifications/unsubscribe-auth/AI7D6TNDFKQZ7S5DAI75BI3TE47CFANCNFSM4ZA4BGBA .