mjs2369 commented 6 months ago

Description:

This PR fixes multiple compile time errors related to fortran standards. For more details on the individual bugs, see the issue pages listed below.

Description of changes:

351

DART/models/wrf/experiments/Radar/IC/sounding_perturbation/pert_sounding_module.f90: Changed the random seed variables (iseed1 and iseed2) from real to integer types

594 / #601

Replaced the use of iargc and getarg with Fortran instrisics COMMAND_ARGUMENT_COUNT() and GET_COMMAND_ARGUMENT() in all possible locations in the code
Removed declaration of the variable for iargc where unused in the code
Removed the f2kcli module and all uses of it in the DART code. This module defined its own subroutines for COMMAND_ARGUMENT_COUNT() and GET_COMMAND_ARGUMENT() that called iargc and getarg

599

DART/models/wrf_hydro/noah_hydro_mod.f90: Simplifies the implementation for summing the number of elements in the CH_NETRT array that are greater than or equal to 0 by using the count function

352

Added specific mkmf.templates for the observation converters AIRS and quikscat, since they use specific configurations with the HDF and/or RTTOV libraries. Versions created for both the gfortran and ifort compilers. The templates that use gfortran make use of the -fallow-argument-mismatch flag to fix type mismatch errors in JPL code
NSIDC no longer produces the error described with the updated mkmf.template.rttov.gfortran file that will be merged with the rttov-clouds branch that makes use of the HDF5 library
The errors produced in the build for GSI2DART have been determined to be out of the scope of the fortran standards update as the entirety of the MPI implementation in this code needs refactored/fixed.

Fixes issue

Fixes #351 #352 #594 #601 #599

Types of changes

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] Documentation update

Documentation changes needed?

[x] My change requires a change to the documentation.
- [x] I have updated the documentation accordingly.

Tests

#351 Compiled successfully with gfortran 12.1.0 (the error was replicated with this version of gfortran before changes were made)

Compiled and run with intel, fixing the value of the seeds and comparing with the output on main. Due to changing the seeds from reals to ints, the changes are not bitwise with a fixed seed. Previous experiments that used real types for the seeds will not produce the same output when using the same value fixed for the seeds as integers.

Two executions with integer types for the seeds produce identical output when fixing the value of the seed.

#594 / #601 Compiled all of the following with cce and gfortran with full debug flags.

models/wrf/WRF_DART_utilities/add_pert_where_high_refl - add_pert_where_high_refl was run and produced correct outputs models/wrf/WRF_DART_utilities/advance_cymdh - Executed the program, which was bitwise identical with main models/wrf/WRF_DART_utilities/grid_refl_obs models/wrf/WRF_BC/update_wrf_bc and models/wrf/WRF_BC/pert_wrf_bc models/wrf/WRF_BC/update_wrf_bc and models/wrf/WRF_BC/update_wrf_bc observations/obs_converters/AVISO/convert_aviso.f90 observations/obs_converters/NCEP/prep_bufr/convert_bufr/arg_test.f and observations/obs_converters/NCEP/prep_bufr/convert_bufr/stat_test.f: observations/obs_converters/NCEP/prep_bufr/convert_bufr/arg_test.f

Note that observations/obs_converters/NCEP/prep_bufr/convert_bufr/grabbufr.f will not compile due to a separate issue.

#595 Both Noah and wrf_hydro compile without error with cce and gfortran with full debug flags A test case is needed in order to run either model

#352 All observation converters (with the exception of GSI2DART, see above) compiled successfully using new mkmf.template files for both gfortran and intel

Checklist for merging

[ ] Updated changelog entry
[ ] Documentation updated
[ ] Update conf.py

Checklist for release

[ ] Merge into main
[ ] Create release from the main branch with appropriate tag
[ ] Delete feature-branch

Testing Datasets

[ ] Dataset needed for testing
[ ] Dataset download instructions included
[ ] No dataset needed

To execute a program that uses the noah_hydro_mod, a test case will need to be provided for for either Noah or wrf_hydro

hkershaw-brown commented 6 months ago

@mjs2369 this is good stuff. I'd like put this in v11 and as v10.X.X tag also. Let's chat about this. Cheers, Helen

mjs2369 commented 6 months ago

I'd like put this in v11 and as v10.X.X tag also

@hkershaw-brown would you like me to go ahead and take this off draft then and move the outstanding issues (#352 and #595) to a later pull request?

hkershaw-brown commented 6 months ago

@mjs2369 no, no rush, no need to split the request, the fortran standards fixes can go in together as one pull request. I just think it will benefit us in the future for comparing versions of dart it will be useful to have a v10 that is compatible with the latest compilers. This note is to remind us (me) to make a v10.10.2 when we put this in to v11.

hkershaw-brown commented 6 months ago

@mjs2369 I'm rebasing this pull request on v11

nancycollins commented 5 months ago

My only main concern is the prepbuffr code. This is not dart code, and has been fragile in the past. So I don't recommend we change this code without having extensive tests (similar to the JPL module code). This code is not built with the dart build system (mkmf, quickbuild). There's these two scripts for building. https://github.com/NCAR/DART/blob/main/observations/obs_converters/NCEP/prep_bufr/install.sh https://github.com/NCAR/DART/blob/main/observations/obs_converters/NCEP/prep_bufr/convert_bufr/convert_bufr.csh

Additionally, there is this "Note that observations/obs_converters/NCEP/prep_bufr/convert_bufr/grabbufr.f will not compile due to a separate issue." So I'd say we treat the prepbur code separately to DART code, and not have prepbuffr changes in this pull request.

i'd agree with this, as long as there's an open issue on updating and testing the prep_bufr code at some point. it's an important converter - it generates all the conventional obs used in the reanalysis and other atmospheric runs.

mjs2369 commented 5 months ago

My only main concern is the prepbuffr code. This is not dart code, and has been fragile in the past. So I don't recommend we change this code without having extensive tests (similar to the JPL module code). This code is not built with the dart build system (mkmf, quickbuild). There's these two scripts for building. https://github.com/NCAR/DART/blob/main/observations/obs_converters/NCEP/prep_bufr/install.sh https://github.com/NCAR/DART/blob/main/observations/obs_converters/NCEP/prep_bufr/convert_bufr/convert_bufr.csh

Additionally, there is this "Note that observations/obs_converters/NCEP/prep_bufr/convert_bufr/grabbufr.f will not compile due to a separate issue." So I'd say we treat the prepbur code separately to DART code, and not have prepbuffr changes in this pull request.

I agree with this and @nancycollins comment. There is currently not an open issue on updating and testing the prep_bufr code, but I will make one and remove these changes from this PR.

hkershaw-brown commented 5 months ago

Derecho: mpif90 failing for gfortran gcc/13.2.0

hkershaw@dec1763:/glade/derecho/scratch/hkershaw/test_code/mpi$ mpif90 hello.f90 
Error: A PrgEnv-* modulefile must be loaded.
hkershaw@dec1763:/glade/derecho/scratch/hkershaw/test_code/mpi$ cat hello.f90 
program hello_world
use mpi

integer :: rank, comm_size, ierr, tag, status(MPI_STATUS_SIZE)

call mpi_init(ierr)
call mpi_comm_size(mpi_comm_world, comm_size, ierr)
call mpi_comm_rank(mpi_comm_world, rank, ierr)

print*, 'Hello from rank ', rank, ' of ', comm_size
call mpi_finalize(ierr)

end program 
hkershaw@dec1763:/glade/derecho/scratch/hkershaw/test_code/mpi$ module list

Currently Loaded Modules:
  1) ncarenv/23.09 (S)   2) craype/2.7.23   3) gcc/13.2.0   4) ncarcompilers/1.0.0   5) hdf5/1.14.3   6) netcdf/4.9.2   7) cray-mpich/8.1.27

  Where:
   S:  Module is Sticky, requires --force to unload or purge

hkershaw-brown commented 5 months ago

MITgcm failing for utilities::nc_check

/glade/derecho/scratch/hkershaw/build_everything/ifort.2024-02-12T0725/DART/models/MITgcm_ocean/model_mod.f90(25): error #6580: Name in only-list does not exist or is not accessible.   [NC_CHECK]
                                  logfileunit, get_unit, nc_check, do_output, to_upper, &\

This is failing on main, fixing it on this branch.

hkershaw-brown commented 5 months ago

Derecho h4fc error:

h4fc -O -assume buffered_io -I/glade/u/apps/derecho/23.09/spack/opt/spack/netcdf/4.9.2/oneapi/2023.2.1/yzvj/include  -c /glade/derecho/scratch/hkershaw/DART/pull_reqeuests/pull_619/DART.converters/assimilation_code/modules/utilities/types_mod.f90
[spack cc] ERROR: Spack compiler must be run from Spack! Input 'SPACK_ENV_PATH' is missing.
make: *** [Makefile:14: types_mod.o] Error 1

To work around this, not using h4fc and putting the linking flags in mkmf.template -lmfhdf -ldf -lsz -ljpeg -lz -ltirpc

hkershaw-brown commented 5 months ago

Hi Marlee, this looks great. I've filed a ticket with CISL about the spack h4fc issue. I think if it is not a quick fix (today), we can add the flags -lmfhdf -ldf -lsz -ljpeg -lz -ltirpc in the AIRS and quickscat mkmf.templates and take out the h4fc

mjs2369 commented 5 months ago

I've filed a ticket with CISL about the spack h4fc issue. I think if it is not a quick fix (today), we can add the flags -lmfhdf -ldf -lsz -ljpeg -lz -ltirpc in the AIRS and quickscat mkmf.templates and take out the h4fc

Sounds good, keep me posted.

Derecho: mpif90 failing for gfortran gcc/13.2.0


hkershaw@dec1763:/glade/derecho/scratch/hkershaw/test_code/mpi$ mpif90 hello.f90 
Error: A PrgEnv-* modulefile must be loaded.
hkershaw@dec1763:/glade/derecho/scratch/hkershaw/test_code/mpi$ cat hello.f90 
program hello_world
use mpi

integer :: rank, comm_size, ierr, tag, status(MPI_STATUS_SIZE)

call mpi_init(ierr)
call mpi_comm_size(mpi_comm_world, comm_size, ierr)
call mpi_comm_rank(mpi_comm_world, rank, ierr)

print*, 'Hello from rank ', rank, ' of ', comm_size
call mpi_finalize(ierr)

end program 
hkershaw@dec1763:/glade/derecho/scratch/hkershaw/test_code/mpi$ module list

Currently Loaded Modules:
  1) ncarenv/23.09 (S)   2) craype/2.7.23   3) gcc/13.2.0   4) ncarcompilers/1.0.0   5) hdf5/1.14.3   6) netcdf/4.9.2   7) cray-mpich/8.1.27

@hkershaw-brown I've never seen this before, did they do some updates to the gcc module while it was down last week? I tried to replicate this error, but I don't even have gcc/13.2.0 available in my modules. Just 12.2.0, which doesn't give this error.

Would this hello code compile with 'use mpif08'?

hkershaw-brown commented 5 months ago

gcc/13 is the default for me when I log in.

hkershaw@derecho6:/glade/derecho/scratch/hkershaw/test_code/mpi$ module avail gcc

-------------------------------------------------------------- Compilers and Core Software ---------------------------------------------------------------
   gcc/12.2.0    gcc/13.2.0 (L,D)

  Where:
   L:  Module is loaded
   D:  Default Module

Yeah you're correct it will need mpif08 to compile, but it doesn't even get to compiling. mpif90 -show was giving me Error: A PrgEnv-* modulefile must be loaded.

I think CISL are working on it at the moment, since there's a slightly different error.

hkershaw@derecho6:/glade/derecho/scratch/hkershaw/test_code/mpi$ mpif90 -show Error: Unable to determine compiler version. Make sure that a gnu module is loaded and that GNU_VERSION is defined

mjs2369 commented 5 months ago

gcc/13 is the default for me when I log in.

hkershaw@derecho6:/glade/derecho/scratch/hkershaw/test_code/mpi$ module avail gcc

-------------------------------------------------------------- Compilers and Core Software ---------------------------------------------------------------
   gcc/12.2.0    gcc/13.2.0 (L,D)

  Where:
   L:  Module is loaded
   D:  Default Module

So weird, I don't have it.


masmith@derecho5:/glade/derecho/scratch/masmith> module avail gcc

------------------------------------------------------- Compilers and Core Software ------------------------------------------------------- gcc/12.2.0 (L)

Where: L: Module is loaded

hkershaw-brown commented 5 months ago

So weird, I don't have it.

masmith@derecho5:/glade/derecho/scratch/masmith> module avail gcc

------------------------------------------------------- Compilers and Core Software -------------------------------------------------------
   gcc/12.2.0 (L)

  Where:
   L:  Module is loaded

guess it is cool kids only.

hkershaw-brown commented 5 months ago

Derecho: mpif90 failing for gfortran gcc/13.2.0

hkershaw@dec1763:/glade/derecho/scratch/hkershaw/test_code/mpi$ mpif90 hello.f90 
Error: A PrgEnv-* modulefile must be loaded.
hkershaw@dec1763:/glade/derecho/scratch/hkershaw/test_code/mpi$ cat hello.f90 
program hello_world
use mpi

integer :: rank, comm_size, ierr, tag, status(MPI_STATUS_SIZE)

call mpi_init(ierr)
call mpi_comm_size(mpi_comm_world, comm_size, ierr)
call mpi_comm_rank(mpi_comm_world, rank, ierr)

print*, 'Hello from rank ', rank, ' of ', comm_size
call mpi_finalize(ierr)

end program 
hkershaw@dec1763:/glade/derecho/scratch/hkershaw/test_code/mpi$ module list

Currently Loaded Modules:
  1) ncarenv/23.09 (S)   2) craype/2.7.23   3) gcc/13.2.0   4) ncarcompilers/1.0.0   5) hdf5/1.14.3   6) netcdf/4.9.2   7) cray-mpich/8.1.27

  Where:
   S:  Module is Sticky, requires --force to unload or purge

Update from CISL, gcc 13 mpi fixed on Derecho !

hkershaw-brown commented 5 months ago

CISL fixed the spack for intel, and gcc/12

NCAR / DART

Fortran standards bug-fixes #619

Description:

351

594 / #601

599

352

Fixes issue

Types of changes

Documentation changes needed?

Tests

Checklist for merging

Checklist for release

Testing Datasets