search

NCAR / ccpp-physics

GFS physics for CCPP
Other
56 stars 144 forks source link

Local w3emc module #1070

Open scrasmussen opened 2 months ago

scrasmussen commented 2 months ago

The external w3emc dependency is added as a local module that is built by the project. The w3emc has been modified to use generic interfaces to handle different input types (for example real32 real64) to functions with the same name. The appropriate module use statements have been added.

grantfirl commented 1 month ago

@scrasmussen I think that we need to target ufs-community:ufs/dev first. Can you rebase it off of ufs/dev and put a PR into there? Also, can you try to move the necessary SP code (splat routine) into sfcsub.f? I realize that we can probably get away with just removing the dependency on the SCM side for its own sake (since we're not compiling sfcsub.f anyway), but there was general agreement to get rid of the SP dependency at the same time as the other NCEPlibs.

scrasmussen commented 3 weeks ago

@scrasmussen I think that we need to target ufs-community:ufs/dev first. Can you rebase it off of ufs/dev and put a PR into there? Also, can you try to move the necessary SP code (splat routine) into sfcsub.f? I realize that we can probably get away with just removing the dependency on the SCM side for its own sake (since we're not compiling sfcsub.f anyway), but there was general agreement to get rid of the SP dependency at the same time as the other NCEPlibs.

@grantfirl Building with ufs-community:ufs/dev revealed additional w3emc library calls that aren't used in the SCM build. I originally went down the route of adding those subroutines and the subroutines they call to the local w3emc module. The problem is that eventually compiling using modern Fortran practices led to a place where subroutines are being called with the wrong number of arguments. I'm guessing that this hasn't been an issue before because the linking phase isn't checking the number of arguments and so it never broke. I decided to not continue down that route since I didn't want to "guess" what the correct behavior would be when less than expected arguments are used and also wasn't sure how many more functions I would need to add (possibly a very large number).

A slightly more hacky solution is to link the w3emc in ufs-weather-model/FV3/ccpp/CMakeLists.txt, thus keeping w3ecm as a dependency for the UFS but removing it for CCPP-Physics. Hopefully this is an ok solution, the only CCPP-Physics files that need w3ecm seem to be in ufs-weather-modell/FV3/ccpp/physics/physics/Interstitials/UFS_SCM_NEPTUNE/

I'll start preparing that PR.

dustinswales commented 2 weeks ago

@scrasmussen I'm confused. It appears that you have all that the CCPP needs in tools/w3emc.F90. No? w3emc dependencies in ccpp physics:

This set is self-contained and doesn't use any other parts of the w3emc library.

scrasmussen commented 2 weeks ago

@dustinswales the fact that the external w3emc routines aren't in a module that is easy to grep makes them harder to track down, that is unless you're compiling with the breaking host model. For example when compiling UFS we have

Then it is adding the dependencies of the getgb, getgbh procedures that leads me down the rabbit-hole, eventually to compiler errors from argument mismatches. I still need to try your idea of using compiler flags to try and get past this issue

dustinswales commented 2 weeks ago

I understand now. Thanks

dustinswales commented 2 weeks ago

A slightly more hacky solution is to link the w3emc in ufs-weather-model/FV3/ccpp/CMakeLists.txt, thus keeping w3ecm as a dependency for the UFS but removing it for CCPP-Physics. Hopefully this is an ok solution, the only CCPP-Physics files that need w3ecm seem to be in ufs-weather-modell/FV3/ccpp/physics/physics/Interstitials/UFS_SCM_NEPTUNE/

@scrasmussen Now that I understand, sorry :), I don't think this is hacky. Starting from a world where the ccpp-physics only requires fortran, netcdf, and mpi, here we have a host-specific interstitial scheme, GFS_phys_time_vary.fv3.F90, with an additional library dependency. Additionally, this dependency is required in the host for which the interstitial physics scheme is utilized. So I think your solution make perfect sense.

@grantfirl @ligiabernardet @lulinxue The raises the question if host-specific interstitial schemes should be given extra freedoms (i.e. no restrictions on external dependencies), compared to the model-agnostic parameterizations (i.e. only fortran, netcdf, mpi)? With the ccpp-physics repository organization, I think this distinction is clear and this should be allowable, but I'd like to see what others think? Another (no so great) option would be for host-specific interstitials to live with the host (e.g. fv3atm/ccpp/Interstitials and scm/src/Interstitials), but since we are sharing the same physics coupling across three hosts this would be a nightmare.