search

GEOS-ESM / GOCART

GOCART Aerosol model including process library and framework interfaces (MAPL, NUOPC, and CCPP)
Apache License 2.0
15 stars 14 forks source link

Workaround for MAPL dependencies #254

Closed climbfuji closed 1 year ago

climbfuji commented 1 year ago

There is a long discussion in the ufs-weather-model (https://github.com/ufs-community/ufs-weather-model/issues/1854) and in MAPL (https://github.com/GEOS-ESM/MAPL/pull/2320) on missing (implicit) dependencies for MAPL. It's been fixed in MAPL develop, but this version won't be used in the software stack for a while.

Meanwhile, we need to find the dependencies for GOCART in spack-stack-1.5.0 explicitly. Because of the QUIET attribute, this won't break other software environments where one or more of fargparse, yafyaml, pflogger are not installed.

mathomp4 commented 1 year ago

Hopefully, MAPL 2.40.4 (and newer) obviate the need for this, but this lets it work with older versions. And, well, this is tested and as far as I can see, cannot hurt even when newer MAPL is tested.

mathomp4 commented 1 year ago

@climbfuji I merged in latest develop and added a changelog entry for you.

climbfuji commented 1 year ago

Ohhh ok, that's a problem if the ufs-weather-model is not at the top of develop at the moment ... do you have a feeling of what happens if we fast-forward the GOCART submodule pointer in the UFS?

mathomp4 commented 1 year ago

Ohhh ok, that's a problem if the ufs-weather-model is not at the top of develop at the moment ... do you have a feeling of what happens if we fast-forward the GOCART submodule pointer in the UFS?

Ohhh. Do you know what version you are using?

climbfuji commented 1 year ago

hash 6ea78fd79037b31a1dcdd30d8a315f6558d963e4

mathomp4 commented 1 year ago

@climbfuji Well, it looks like that is essentially our main and now the only difference is fixes from #253 for FENGSHA that just got in:

https://github.com/GEOS-ESM/GOCART/compare/6ea78fd...develop

So it is at most no change and if not, better, if you are running FENGSHA.

mathomp4 commented 1 year ago

If you are okay with that, I can work with @vbuchard to get your fix into develop as well (since it cannot affect our way of building GOCART at all)

climbfuji commented 1 year ago

Should we change the target branch to main then and revert the merge from develop? Or merge this as-is into develop and then cherry-pick for another PR to main?

bbakernoaa commented 1 year ago

We will need the latest develop branch as there is a need from a scientific standpoint for the dust scheme. I ran a test with the latest develop and it works fine with the current version of MAPL. https://github.com/ufs-community/ufs-weather-model/pull/1922

climbfuji commented 1 year ago

That's great news, thanks @bbakernoaa. @mathomp4 How about we merge this now, and then I'll either wait for @bbakernoaa 's PR to go in first, or I update the submodule pointer in my ufs-weather-model PR?

bbakernoaa commented 1 year ago

@climbfuji if you want to include the submodule update in your PR that is fine with me. We just need to get this moving for the EP5 experiments.

climbfuji commented 1 year ago

@bbakernoaa To confirm, do I need the changes in tests/parm/gocart/DU2G_instance_DU.rc from your PR?

https://github.com/ufs-community/ufs-weather-model/pull/1922/files

I am getting segfaults without them when I run the cpld_control_p8_intel test on Hera (spack-stack-1.5.0).

137: pe=00137 FAIL at line=00187    DU2G_GridCompMod.F90                     <status=41>
137: pe=00137 FAIL at line=04602    MAPL_Generic.F90                         <status=41>
137: pe=00137 FAIL at line=04789    MAPL_Generic.F90                         <status=41>
137: pe=00137 FAIL at line=01335    GOCART2G_GridCompMod.F90                 <status=41>
121: pe=00121 FAIL at line=00187    DU2G_GridCompMod.F90                     <status=41>
123: pe=00123 FAIL at line=00187    DU2G_GridCompMod.F90                     <status=41>
...
mathomp4 commented 1 year ago

@climbfuji That looks to me like the answer is yes. ESMF is complaining that it can't satisfy soil_moisture_factor since the call has no default:

       call ESMF_ConfigGetAttribute (cfg, self%f_swc,      label='soil_moisture_factor:', __RC__)
climbfuji commented 1 year ago

@climbfuji That looks to me like the answer is yes. ESMF is complaining that it can't satisfy soil_moisture_factor since the call has no default:

       call ESMF_ConfigGetAttribute (cfg, self%f_swc,      label='soil_moisture_factor:', __RC__)

Thanks, I cherry-picked that commit in my branch so that credits go to @bbakernoaa and not me, testing now.

bbakernoaa commented 1 year ago

@climbfuji Yes you will need those. Otherwise the GOCART component will crash. The attributes must be present in .rc file.