Update NEMS for CDEPS component #92

binli2337 · 2021-03-11T19:22:28Z

The existing data atmosphere model component will be renamed "nems_data".
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

…ents.

uturuncoglu · 2021-03-12T16:49:45Z

src/module_EARTH_GRID_COMP.F90

@@ -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. &


Are these CDEPS data atmosphere? I think this implementation is not flexible enough because you need to add a line for each specific datm stream. At this level model only need to know the datm not the stream used in CDEPS. So, only datm is enough for driver at this point. Please refer to following version of the file found in hafs-community#3

Yes, we need to remove datm_cfsr and datm_gefs because the way to configure the DATM input data is via its namelist. Moreover, the driver does not need to know what the source of the data is. Also, this approach means we'd need to change the driver to add a new data source to CDEPS. All good reasons why we only need DATM at the driver level. This is important to get right.

@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.

uturuncoglu · 2021-03-12T16:50:10Z

src/module_EARTH_GRID_COMP.F90

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


This is same with the previous one.

junwang-noaa · 2021-03-18T16:53:56Z

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: ***@***.**** commented on this pull request. ------------------------------ In src/module_EARTH_GRID_COMP.F90 <#92 (comment)>: > @@ -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 <#92 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AI7D6TKNCPFXORDYL2UPKPLTEIVCDANCNFSM4ZA4BGBA> .

uturuncoglu · 2021-03-18T16:55:12Z

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 <#92 (comment)>: > @@ -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 <#92 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJMBR4TN2ROPEJTH6GYDO3TEIVCDANCNFSM4ZA4BGBA>.

rsdunlapiv · 2021-03-18T16:55:34Z

@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 · 2021-03-18T17:03:46Z

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 · 2021-03-18T17:31:26Z

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 · 2021-03-18T18:19:32Z

@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 · 2021-03-18T19:28:25Z

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 <#92 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AI7D6TN4OH7LUNXQ4RPHA4DTEI74TANCNFSM4ZA4BGBA> .

binli2337 · 2021-03-21T20:00:54Z

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

* The name of the NEMS data atmosphere model is changed to nems_datm. * The name of the CDEPS atmosphere model will be datm.

uturuncoglu · 2021-03-22T04:30:39Z

@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 · 2021-03-22T13:13:52Z

@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 · 2021-03-22T13:23:14Z

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 <#92 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AI7D6TNDFKQZ7S5DAI75BI3TE47CFANCNFSM4ZA4BGBA> .

uturuncoglu · 2021-03-22T15:23:27Z

src/module_EARTH_GRID_COMP.F90

-          elseif (trim(model) == "datm") then
-#ifdef FRONT_DATM
+          elseif (trim(model) == "nems_datm") then
+#ifdef FRONT_NEMS_DATM


@binli2337 @junwang-noaa I think we don't need to have same code twice in here. You could use #if defined FRONT_NEMS_DATM || defined FRONT_CDEPS_DATM and remove extra if statement in here. This is already working under HAFS fork.

Revised module_EARTH_GRID_COMP.F90 for datm_cfsr and datm_gefs compon…

1b4a5bf

…ents.

binli2337 mentioned this pull request Mar 12, 2021

Add CDEPS component, add CDEPS tests and fix bugs in the datm_bulk_gefs test ufs-community/ufs-weather-model#471

Closed

uturuncoglu suggested changes Mar 12, 2021

View reviewed changes

MinsukJi-NOAA requested review from junwang-noaa and DeniseWorthen March 18, 2021 16:24

Revised module_EARTH_GRID_COMP.F90.

5e75382

* The name of the NEMS data atmosphere model is changed to nems_datm. * The name of the CDEPS atmosphere model will be datm.

uturuncoglu reviewed Mar 22, 2021

View reviewed changes

binli2337 added 2 commits March 23, 2021 14:52

Updated module_EARTH_GRID_COMP.F90.

fc4308c

Merge branch 'develop' into feature/new_cdeps

62364ab

binli2337 changed the title ~~Revised module_EARTH_GRID_COMP.F90 for datm_cfsr and datm_gefs compon…~~ Update NEMS for CDEPS component Apr 21, 2021

binli2337 mentioned this pull request Apr 22, 2021

Update NEMS and CMEPS, add CDEPS tests, fix bugs in the datm_bulk_gefs test ufs-community/ufs-weather-model#538

Merged

4 tasks

junwang-noaa approved these changes Apr 22, 2021

View reviewed changes

binli2337 added 3 commits April 25, 2021 18:42

Merge branch 'develop' into feature/new_cdeps

ca47b0a

Merge branch 'develop' into feature/new_cdeps

8622809

Merge branch 'develop' into feature/new_cdeps

6a78eec

DeniseWorthen approved these changes May 4, 2021

View reviewed changes

MinsukJi-NOAA merged commit b593c6c into NOAA-EMC:develop May 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update NEMS for CDEPS component #92

Update NEMS for CDEPS component #92

binli2337 commented Mar 11, 2021 •

edited

Loading

uturuncoglu Mar 12, 2021

rsdunlapiv Mar 18, 2021

DeniseWorthen Mar 18, 2021

uturuncoglu Mar 12, 2021

junwang-noaa commented Mar 18, 2021 via email

uturuncoglu commented Mar 18, 2021 via email

rsdunlapiv commented Mar 18, 2021

DeniseWorthen commented Mar 18, 2021

binli2337 commented Mar 18, 2021 •

edited

Loading

DeniseWorthen commented Mar 18, 2021

junwang-noaa commented Mar 18, 2021 via email

binli2337 commented Mar 21, 2021

uturuncoglu commented Mar 22, 2021 •

edited

Loading

binli2337 commented Mar 22, 2021

junwang-noaa commented Mar 22, 2021 via email

uturuncoglu Mar 22, 2021

Update NEMS for CDEPS component #92

Update NEMS for CDEPS component #92

Conversation

binli2337 commented Mar 11, 2021 • edited Loading

uturuncoglu Mar 12, 2021

Choose a reason for hiding this comment

rsdunlapiv Mar 18, 2021

Choose a reason for hiding this comment

DeniseWorthen Mar 18, 2021

Choose a reason for hiding this comment

uturuncoglu Mar 12, 2021

Choose a reason for hiding this comment

junwang-noaa commented Mar 18, 2021 via email

uturuncoglu commented Mar 18, 2021 via email

rsdunlapiv commented Mar 18, 2021

DeniseWorthen commented Mar 18, 2021

binli2337 commented Mar 18, 2021 • edited Loading

DeniseWorthen commented Mar 18, 2021

junwang-noaa commented Mar 18, 2021 via email

binli2337 commented Mar 21, 2021

uturuncoglu commented Mar 22, 2021 • edited Loading

binli2337 commented Mar 22, 2021

junwang-noaa commented Mar 22, 2021 via email

uturuncoglu Mar 22, 2021

Choose a reason for hiding this comment

binli2337 commented Mar 11, 2021 •

edited

Loading

binli2337 commented Mar 18, 2021 •

edited

Loading

uturuncoglu commented Mar 22, 2021 •

edited

Loading