Note, sets "USE_PIO2=true" for this case as target option.

cc @xylar

@pwolfram it seems worrisome to have only one configuration with USE_PIO2=true, it may cause unexpected issues for other users. We should make sure the broader team is on board with that change.

looking closer it seems you are adding a new configuration right? Then I'm fine with the PIO2 change. Disregard the previous comment

Yes, @vanroekel, only specifying it for that specific build, not general for everyone.

ps @vanroekel , please leave a review.

sure @pwolfram will test this stack in the advection convergence test case and chime in after

The new intel-mpi target appears identical to the existing ifort target in all but two respects:

1. The names of the MPI compiler wrappers are Intel-specific in the former; and
2. the USE_PIO2 option is hard-wired to true in the former.

Regarding the hard-wiring of USE_PIO2=true, I don't really like the idea of having any target hard-wire any of the options listed in the Makefile, as it defeats the purpose of options and makes the target less general than it needs to be. It also doesn't seem that difficult to ask users to specify USE_PIO2=true when compiling on a particular machine; we have decent checks in the Makefile that should even tell users if they need to add the USE_PIO2=true option.

Regarding the Intel-specific compiler wrappers, my general concern is that we're setting a precedent that will support the proliferation of build targets to support every possible variation in build environment. I'm open to arguments in favor of the intel-mpi target, but it's probably worth asking whether the modules on grizzly could be set up to let mpif90 be a symlink or alias for mpiifort, and similarly for mpicc and mpiicc? Another option to consider is whether the Intel MPI implementation provides a Fortran wrapper named mpifort (which, I understand, is the more modern version of mpif90), and whether we could simply update the existing ifort target to use mpifort rather than mpif90?

@pwolfram note that following the comment from @mgduda I tried building on grizzly with your stack and with

make ifort CORE=ocean USE_PIO2=true

This did fail. However, if I did

export I_MPI_F90=ifort

first then the model built fine without the new configuration. I would suggest we remove intel-mpi and just add the export line to our individual scripts to load modules on grizzly.

note the solution for building was found here -- https://stackoverflow.com/questions/32335519/gfortran-is-called-instead-of-mpif90

The general version of it from https://software.intel.com/en-us/forums/intel-clusters-and-hpc-technology/topic/288354 is

Note that if you want to use mpif90/mpicc/etc (eg for existing configure scripts/Makefiles, etc) with the Intel compilers, one need only modify environment variables to point to the appropriate compilers: eg,
export I_MPI_CC=icc
 export I_MPI_CXX=icpc
 export I_MPI_F77=ifort
 export I_MPI_F90=ifort
for bash-type shells, or
setenv I_MPI_CC icc
setenv I_MPI_CXX icpc
setenv I_MPI_F77 ifort
setenv I_MPI_F90 ifort
for csh-type shells.

In sum, consensus is I should remove the intel-mpi target (done). I've tested this works by using these load-up settings on grizzly:

module purge 
# assumes python2, so this doesn't work                                                                                                                         
#source /usr/projects/climate/SHARED_CLIMATE/anaconda_envs/load_latest_compass.sh                                                      
module use /usr/projects/climate/SHARED_CLIMATE/modulefiles/all/                                                                      
module load friendly-testing                                                                                                          
module load intel/19.0.4 intel-mpi/2019.4 hdf5-parallel/1.8.16 pnetcdf/1.11.2 netcdf-h5parallel/4.7.3 mkl/2019.0.4 scorpio/pio2/1.10.1
export I_MPI_CC=icc                                                                                                                   
export I_MPI_CXX=icpc                                                                                                                 
export I_MPI_F77=ifort                                                                                                                
export I_MPI_F90=ifort                                                                                                                

and build with

make ifort CORE=ocean USE_PIO2=true

Thanks @xylar! Everyone, changes made and PR ready to move forward with other review acceptances.

@mgduda, any further comments? The PR now only points to lib or lib64, whichever is available.

@matthewhoffman any comments?

@mgduda, removed

echo "loading modules or grizzly"
module purge 
# assumes python2, so this doesn't work                                                                                                                         
#source /usr/projects/climate/SHARED_CLIMATE/anaconda_envs/load_latest_compass.sh                                                      
module use /usr/projects/climate/SHARED_CLIMATE/modulefiles/all/                                                                      
module load friendly-testing                                                                                                          
module load intel/19.0.4 intel-mpi/2019.4 hdf5-parallel/1.8.16 pnetcdf/1.11.2 netcdf-h5parallel/4.7.3 mkl/2019.0.4 scorpio/pio2/1.10.1
export I_MPI_CC=icc                                                                                                                   
export I_MPI_CXX=icpc                                                                                                                 
export I_MPI_F77=ifort                                                                                                                
export I_MPI_F90=ifort 

to build with

make ifort CORE=ocean USE_PIO2=true

from PR descriptions and commit messages as requested.

@pwolfram Thanks for the updates to the PR description. It's not clear to me how the changes are related to PIO2, which is specifically mentioned in the PR description. It looks to me like the changes affect the netCDF and parallel-netCDF libraries without regard to which version of PIO is used. Or is it that only PIO2 can use netCDF libraries in lib64? It might also make the PR description more complete to mention that the lib versus lib64 affects just the netCDF and parallel-netCDF libraries, and to point out which of lib or lib64 will take precedence if both were to somehow be found. Of course, most people would have no trouble figuring this out by looking at the code diffs, but it seems not a bad practice to be thorough in descriptions of code changes, especially in terms of the why (the what is usually apparent from the diffs, but the motivation isn't always so clear).

Thanks @mgduda, please see updated descriptions following your feedback.

@pwolfram thanks for your work on this.