Hotfix - allow the latest MPAS-Model v8.4.1-1.3 build successfully

[guoqing-noaa]

allow the latest MPAS-Model v8.4.1-1.3 build successfully

• add needed compiling dependencies in stochastic_physics
• detect NETCDF_C_ROOT and NETCDF_FORTRAN_ROOT automatically for the make system, to be consistent with the CMake system

This PR would like to update the stochastic_physics submodule hash if possible,
which depends on this PR: dtcenter/stochastic_physics#9

Resolve issue #276

Mandatory Questions

• Does this PR include any additions or changes to external inputs (e.g., microphysics lookup tables, static data for gravity-wave drag -- things like that)?
  • no
• Does this PR require updating one or more baselines for the CI tests? If so, what?
  • no

Reviews

• Is this PR currently draft, still being worked on, or ready for review?
  • ready for review
• For when this PR begins review, please list the developers/collaborators you'd like to prioritize for review; e.g.,:
  • @dustinswales
  • @clark-evans

[clark-evans]

This looks good to me, but I will hold off on a formal "approve" until there is resolution on the stochastic_physics PR that this builds upon.

[guoqing-noaa]

Note: This PR alone does not solve all the building issues. We will need PR: dtcenter/stochastic_physics#9 as well.

For those who want to use a workable version now, follow these steps:

git clone -b v8.4.1-1.3x --recursive git@github.com:RRFSx/MPAS-Model
cd MPAS-Model
module use modulefiles
module load mpas/ursa
make -j8 intel-mpi-ursa CORE=atmosphere

[gsketefian]

@guoqing-noaa Yeah, this sounds good. I added these variables in my module files in the (now discontinued) mpas_app workflow. That's why it was working for me. But they should be added to the Makefile in MPAS-Model.

[dustinswales]

@guoqing-noaa Thanks for these changes, and the changes in the stochastic physics subcomponent #9.

[clark-evans]

Hi @guoqing-noaa - with this PR, should we also update the modules for each supported HPC in the modulefiles/mpas/ directory?

[guoqing-noaa]

Hi @guoqing-noaa - with this PR, should we also update the modules for each supported HPC in the modulefiles/mpas/ directory?

@clark-evans Gerard has incorporated needed changes in his stochastic PR recently merged.

[clark-evans]

Hi @guoqing-noaa - with this PR, should we also update the modules for each supported HPC in the modulefiles/mpas/ directory?

@clark-evans Gerard has incorporated needed changes in his stochastic PR recently merged.

OK, thanks - I wasn't sure if netCDF libraries also needed to be added.

[gsketefian]

@guoqing-noaa In the module files in MPAS-Model/modulefiles/mpas, e.g. in hera.intel.lua, there is this line:

  setenv("NetCDF_C_ROOT", os.getenv("netcdf_c_ROOT"))

So this defines the variable NetCDF_C_ROOT by setting it to netcdf_c_ROOT (in these variable names, case matters). NetCDF_C_ROOT is then used in the code here to set NETCDF, i.e.

NETCDF := $(NetCDF_C_ROOT)

Is there a reason to introduce the intermediate variable NetCDF_C_ROOT? To simplify things, can we just use netcdf_c_ROOT (which is set by the module load commands in the module file) instead here, i.e.

NETCDF := $(netcdf_c_ROOT)

Same for NetCDF_FORTRAN_ROOT.

[guoqing-noaa]

@gsketefian I don't know why spack-stack choosed to use netcdf_c_ROOT , but NetCDF_C_ROOT is the standard environmental variable name at this time if netcdf_c and netcdf_fortran are installed separately. If you check the CMake system, you will notice that it accepts only NetCDF_C_ROOT. So we want the CMake and make systems to be consistent and adhere to the industry standard.

[gsketefian]

Ok.

[gsketefian]

@guoqing-noaa On a separate note, as I was retrying the build with your changes to stochastic_physics, I found a minor bug in the clean process. In MPAS-Model/src/core_atmosphere/Makefile, in the clean: target, after the line

        $(RM) -r libphys

there should be a line like

        $(RM) -r libchem

Right now, a clean leaves the libchem directory still intact. If you like, you can add this bug fix to this PR.

[guoqing-noaa]

@guoqing-noaa On a separate note, as I was retrying the build with your changes to stochastic_physics, I found a minor bug in the clean process. In MPAS-Model/src/core_atmosphere/Makefile, in the clean: target, after the line

        $(RM) -r libphys

there should be a line like

        $(RM) -r libchem

Right now, a clean leaves the libchem directory still intact. If you like, you can add this bug fix to this PR.

Thanks for identifying this issue. Happy to include it here.

[guoqing-noaa]

Thank @gsketefian for testing and merging the stochastic_physics PR and reviewing this PR.
I have updated as suggested.
Tested new compiling and cleaning on Gaea and worked as expected.

@clark-evans I think this PR is good to go now. Thanks!

[AndersJensen-NOAA]

@clark-evans @joeolson42 @JeffBeck-NOAA @dustinswales
I was able to reproduce @guoqing-noaa fixes to get the standalone system to compile (I have not tested run):

1 These Modules are needed:

1. intel-oneapi-compilers/2023.2.0 3) intel-oneapi-mkl/2024.2.1 5) bzip2/1.0.8 7) netcdf-c/4.9.2
2. intel-oneapi-mpi/2021.13.1 4) parallel-netcdf/1.12.3 6) hdf5/1.14.3 8) netcdf-fortran/4.6.1

Note: when loading netcdf-fortran/4.6.1, bzip2, hdf5, and netcdf-c automatically load

2 This Commit is needed:

dtcenter/stochastic_physics@d45e9fb

3 A Gemini-written change to the main MPAS Makefile that uses nf-config to find netcdf.inc.

I think for now, we should roll back this PR. As we discussed, someone should make changes so that stochastic_physics can be turned off completely at compile time, and we can build with our original 3 modules.