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

NEMS driver cleanup #98

Closed DeniseWorthen closed 3 years ago

DeniseWorthen commented 3 years ago

Fixes issue #95

c96dd29: remove NEMS mediator files ac9558c: switch to yaml field dictionary, remove unused/legacy model fronts 8bbeb2a: remove ESMF<8 conditional blocks and code ae01a66: clean up error logging 6f7ea44: simplify error messaging if required component is not found

DusanJovic-NOAA commented 3 years ago

Consider removing module_NEMS_INTERNAL_STATE.F90. It does not contain any useful state, just two dummy variables.

DusanJovic-NOAA commented 3 years ago

Now that we removed ensemble coupler there's no need to have both NEMS and EARTH components, one should be enough. Basically 'nems' should be just a main program and one grid component.

DusanJovic-NOAA commented 3 years ago

I also do not see where two variables in EARTH_INTERNAL_STATE:

        real(ESMF_KIND_R8)  :: medAtmCouplingIntervalSec
        real(ESMF_KIND_R8)  :: medOcnCouplingIntervalSec

are used. If they are not needed anymore, maybe module_EARTH_INTERNAL_STATE.F90 can be removed as well.

DeniseWorthen commented 3 years ago

There will be one more PR to clean up the driver itself but I think removing the NEMS internal state file would be OK at this point.

DusanJovic-NOAA commented 3 years ago

Five subroutines from module_NEMS_UTILS.F90 (err_msg, err_msg_int, err_msg_val, err_msg_var and err_msg_final) are also not used anywhere. CHECK_ESMF_PET can be moved to MAIN_NEMS.F90. Which basically means module_NEMS_UTILS.F90 can be removed.

Functions RTC() and TIMEF() can be removed from MAIN_NEMS.F90

DeniseWorthen commented 3 years ago

I'm going to retain the NEMS_UTILS SR for now since I may want to move some items there in the second round.

I'm not sure what to do about the two INTERNAL_STATE SRs. I'm not confident enough of their purpose to actually remove them.

DusanJovic-NOAA commented 3 years ago

I'm going to retain the NEMS_UTILS SR for now since I may want to move some items there in the second round.

I'm not sure what to do about the two INTERNAL_STATE SRs. I'm not confident enough of their purpose to actually remove them.

None of the variables from two internal states are used. They (internal states) can be removed.

junwang-noaa commented 3 years ago

I'd suggest to check with Mark Potts before removing the NEMS/earth internal state file. I am not sure if the JEDI-NEMS interface has been set up yet and if NEMS internal state will be used in that setting.

DusanJovic-NOAA commented 3 years ago

I'd suggest to check with Mark Potts before removing the NEMS/earth internal state file. I am not sure if the JEDI-NEMS interface has been set up yet and if NEMS internal state will be used in that setting.

I do not see how and why would JEDI access to nems and earth internal states. As the name suggests they are internal. If JEDI needs anything from nems grid component it should access it via import and/or export states.

@mark-a-potts Can you please check if JEDI needs access to NEMS internal state.

mark-a-potts commented 3 years ago

I'd suggest to check with Mark Potts before removing the NEMS/earth internal state file. I am not sure if the JEDI-NEMS interface has been set up yet and if NEMS internal state will be used in that setting.

I do not see how and why would JEDI access to nems and earth internal states. As the name suggests they are internal. If JEDI needs anything from nems grid component it should access it via import and/or export states.

@mark-a-potts Can you please check if JEDI needs access to NEMS internal state.

The plan is for JEDI to get all information via NUOPC imports and exports, so it will not use the internal states.

DeniseWorthen commented 3 years ago

The EARTH_INTERNAL_STATE does appear to be used in the module_EARTH_GRID_COMP (is%EARTH_INT_STATE)?

DusanJovic-NOAA commented 3 years ago

The EARTH_INTERNAL_STATE does appear to be used in the module_EARTH_GRID_COMP (is%EARTH_INT_STATE)?

EARTH_INTERNAL_STATE is declared, allocated, stored in the grid component and finally deallocated. But it's never actually used. Same for nems internal state. They can be removed.

DusanJovic-NOAA commented 3 years ago

Great. Now we just need to merge nems and earth component into one grid component, move check_esmf_pet to main program or use some alternative way of setting log type, figure out what to about these two rusage files and we'll be down to just main program and one grid component module. But that's for the next PR.

MinsukJi-NOAA commented 3 years ago

@DeniseWorthen is this pr ready for commit?

DeniseWorthen commented 3 years ago

Yes it is but we haven't been merging the subcomponent PRs until the ufs-weather is updated w/ the RTs. Did you want to merge it now?

MinsukJi-NOAA commented 3 years ago

@DeniseWorthen Yes let's wait for the ufs-weather RT results.