IOSTAT variable needs initializing

[drakest123]

In share/ioModule.f90 Subroutine read_snow17_parameters(), the IOSTAT variable ios is non-zero upon subsequent calls to this subroutine.

Current behavior

When running multiple catchments, variable ios is set to zero by a compile time setting. However, in subsequent calls to Subroutine read_snow17_parameters() this variable has a non-zero value.

Resolution

Explicitly set ios = 0 after variable declarations

[andywood]

ios is initialized in the variable declarations on each call to the subroutine, then is only assigned by the read statements and checked for each readline to see if one has failed (ie non-zero return value). are you sure this doesn't work?
ln 27: integer :: ios=0

[drakest123]

@andywood the ios assignment works for the initial invocation of read_snow17_parameters() but not successive calls. For example, adding a print statement before the input file is read has ios=-1 during the 2nd invocation:

% ./cmake_build/ngen data/catchment_data.geojson all data/nexus_data.geojson nex-26 data/example_bmi_multi_realization_config_w_snow17.json NGen Framework 0.1.0 Building Nexus collection Building Catchment collection Initializing formulations [ { name : bmi_c++, params : { allow_exceed_end_time : true, init_config : /dev/null, library_file : ./extern/sloth/cmake_build/libslothmodel.so, main_output_variable : z, model_params : { sloth_ice_fraction_schaake(1,double,m,node) : 0, sloth_ice_fraction_xinanjiang(1,double,1,node) : 0, sloth_smp(1,double,1,node) : 0, }, model_type_name : bmi_c++_sloth, uses_forcing_file : false, }, }, { name : bmi_fortran, params : { allow_exceed_end_time : true, forcing_file : , init_config : ./data/bmi/fortran/snow17-init-{{id}}.namelist.input, library_file : ./extern/snow17/cmake_build/libsnow17bmi, main_output_variable : raim, model_type_name : bmi_fortran_snow17, uses_forcing_file : false, variables_names_map : { precip : atmosphere_water__liquid_equivalent_precipitation_rate, tair : land_surface_air__temperature, }, }, }, { name : bmi_c, params : { allow_exceed_end_time : true, forcing_file : , init_config : ./data/bmi/c/pet/{{id}}_bmi_config.ini, library_file : ./extern/evapotranspiration/cmake_build/libpetbmi, main_output_variable : water_potential_evaporation_flux, model_type_name : bmi_c_pet, registration_function : register_bmi_pet, uses_forcing_file : false, variables_names_map : { water_potential_evaporation_flux : potential_evapotranspiration, }, }, }, { name : bmi_c, params : { allow_exceed_end_time : true, forcing_file : , init_config : ./data/bmi/c/cfe/{{id}}_bmi_config.ini, library_file : ./extern/cfe/cmake_build/libcfebmi, main_output_variable : Q_OUT, model_type_name : bmi_c_cfe, registration_function : register_bmi_cfe, uses_forcing_file : false, variables_names_map : { atmosphere_air_water~vapor__relative_saturation : SPFH_2maboveground, ice_fraction_schaake : sloth_ice_fraction_schaake, ice_fraction_xinanjiang : sloth_ice_fraction_xinanjiang, land_surface_air__pressure : PRES_surface, land_surface_air__temperature : TMP_2maboveground, land_surface_radiation~incoming~longwave__energy_flux : DLWRF_surface, land_surface_radiation~incoming~shortwave__energy_flux : DSWRF_surface, land_surface_wind__x_component_of_velocity : UGRD_10maboveground, land_surface_wind__y_component_of_velocity : VGRD_10maboveground, soil_moisture_profile : sloth_smp, water_potential_evaporation_flux : potential_evapotranspiration, }, }, }, ] SUBROUTINE initialize_from_file Calling readNamelist() Reading namelist -- simulating basin cat-67 with 1 snowbands Calling initInfo() Calling initForcing() Calling initModelVar() Calling initParams() Calling read_snow17_parameters() Opening parameter file: extern/snow17/test_cases/ex1/input/params/snow17_params.HHWM7.txt Reading Snow17 parameters
ios: 0
hru_id HHWM8IL HHWM8IU hru_area 2994.7 1271.3 latitude 47.78 47.78 elev 1612.50 2153.35 scf 2.15177 1.86124 mfmax 0.930472 0.754924 mfmin 0.137 0.160 uadj 0.003103 0.208042 si 1515.00 1515.00 pxtemp 0.713424 0.220934 nmf 0.150 0.150 tipm 0.200 0.050 mbase 0.000 0.000 plwhc 0.030 0.030 daygm 0.300 0.200 adc1 0.050 0.050 adc2 0.090 0.090 adc3 0.160 0.160 adc4 0.310 0.310 adc5 0.540 0.540 adc6 0.740 0.740 adc7 0.840 0.840 adc8 0.890 0.890 adc9 0.930 0.930 adc10 0.970 0.970 adc11 1.000 1.000 Config file details - Line Count: 22 | Max Line Length 40 Config Value - Param: 'forcing_file' | Value: 'BMI' | Units: '(null)' Config Value - Param: 'verbosity' | Value: '0' | Units: '(null)' Config Value - Param: 'surface_partitioning_scheme' | Value: 'Schaake' | Units: '(null)' Config Value - Param: 'soil_params.depth' | Value: '2.0' | Units: '(null)' WARNING: [units] expected for 'soil_params.depth' in config file Config Value - Param: 'soil_params.b' | Value: '4.05' | Units: '(null)' Config Value - Param: 'soil_params.mult' | Value: '1000.0' | Units: '(null)' Config Value - Param: 'soil_params.satdk' | Value: '0.00000338' | Units: '(null)' WARNING: [units] expected for 'soil_params.satdk' in config file Config Value - Param: 'soil_params.satpsi' | Value: '0.355' | Units: '(null)' WARNING: [units] expected for 'soil_params.satpsi' in config file Config Value - Param: 'soil_params.slop' | Value: '1.0' | Units: '(null)' WARNING: [units] expected for 'soil_params.slop' in config file Config Value - Param: 'soil_params.smcmax' | Value: '0.439' | Units: '(null)' WARNING: [units] expected for 'soil_params.smcmax' in config file Config Value - Param: 'soil_params.wltsmc' | Value: '0.066' | Units: '(null)' WARNING: [units] expected for 'soil_params.wltsmc' in config file Config Value - Param: 'max_gw_storage' | Value: '16.0' | Units: '(null)' WARNING: [units] expected for 'max_gw_storage' in config file Config Value - Param: 'Cgw' | Value: '0.01' | Units: '(null)' WARNING: [units] expected for 'Cgw' in config file Config Value - Param: 'expon' | Value: '6.0' | Units: '(null)' Config Value - Param: 'gw_storage' | Value: '50%' | Units: '(null)' WARNING: [units] expected for 'gw_storage' in config file Config Value - Param: 'alpha_fc' | Value: '0.33' | Units: '(null)' Config Value - Param: 'soil_storage' | Value: '66.7%' | Units: '(null)' WARNING: [units] expected for 'soil_storage' in config file Config Value - Param: 'K_lf' | Value: '0.01' | Units: '(null)' Config Value - Param: 'K_nash' | Value: '0.03' | Units: '(null)' Config Value - Param: 'nash_storage' | Value: '0.0,0.0' | Units: '(null)' Config Value - Param: 'giuh_ordinates' | Value: '0.06,0.51,0.28,0.12,0.03' | Units: '(null)' Found configured GIUH ordinate values ('0.06,0.51,0.28,0.12,0.03') Config Value - Param: 'num_timesteps' | Value: '720' | Units: '(null)' Config param 'soil_params.expon' not found in config file, defaulting to 1 (linear) Config param 'soil_params.expon_secondary' not found in config file, defaulting to 1 (linear) Schaake Magic Constant calculated All CFE config params present GIUH ordinates string value found in config ('0.06,0.51,0.28,0.12,0.03') Counted number of GIUH ordinates (5) Finished function parsing CFE config At declaration of smc_profile size, soil_reservoir.n_soil_layers = 0 SUBROUTINE initialize_from_file Calling readNamelist() Reading namelist -- simulating basin cat-27 with 1 snowbands Calling initInfo() Calling initForcing() Calling initModelVar() Calling initParams() Calling read_snow17_parameters() Opening parameter file: extern/snow17/test_cases/ex1/input/params/snow17_params.HHWM8.txt Reading Snow17 parameters
ios: -1
Read 0 SNOW17 params, but need 26. Quitting.

[andywood]

ok, that suggests the line 27 initialization doesn't work. I wonder if that is compiler dependent? (e.g., declaration+assignment is not supported). Anywhere between line 27 and 37, every time the subroutine is called, ios should have a value of 0.

[SnowHydrology]

Yes, assignment at declaration is usually a no-no in Fortran. A simple change to the following should suffice.

integer :: ios
ios = 0

@drakest123 do you see assignment at declaration anywhere else in the code?

Additionally, can someone explain line 41 to me:

    do while(ios .eq. 0)
      read(unit=51,FMT='(A)',IOSTAT=ios) readline
  
      if(ios == 0) then

If the do loop only runs when ios == 0, what purpose does the if(ios == 0) then serve?

[andywood]

I think the practice of assigning simple vars at declaration is not uncommon - maybe grew out of card reader days when you could save a line by doing so. after googling it, I do see it's frowned upon. we can add revising these assignments where we find them to the code cleanup list if it's not already there.

I think the logic you mention is a clunky way to exit the case loop when you hit the end of the file (or some other error) -- ie the last attempt to read a line fails, no cases are triggered, and you don't go through it again. perhaps otherwise you need a break statement. I agree there are other ways you could set this up.

[andywood]

btw -- that is in other subroutines as well, even in that same module (and it's scattered through SUMMA, fwiw; also in some of the orig noah-mp code)

[drakest123]

Thanks for the heads-up. I'll look for other instances of this issue. After that, it would be good to compare the output with a previous version. FYI, I'm using gfortran on a Mac M1:
% gfortran -v Using built-in specs. COLLECT_GCC=gfortran COLLECT_LTO_WRAPPER=/opt/homebrew/Cellar/gcc/13.2.0/bin/../libexec/gcc/aarch64-apple-darwin23/13/lto-wrapper Target: aarch64-apple-darwin23 Configured with: ../configure --prefix=/opt/homebrew/opt/gcc --libdir=/opt/homebrew/opt/gcc/lib/gcc/current --disable-nls --enable-checking=release --with-gcc-major-version-only --enable-languages=c,c++,objc,obj-c++,fortran --program-suffix=-13 --with-gmp=/opt/homebrew/opt/gmp --with-mpfr=/opt/homebrew/opt/mpfr --with-mpc=/opt/homebrew/opt/libmpc --with-isl=/opt/homebrew/opt/isl --with-zstd=/opt/homebrew/opt/zstd --with-pkgversion='Homebrew GCC 13.2.0' --with-bugurl=https://github.com/Homebrew/homebrew-core/issues --with-system-zlib --build=aarch64-apple-darwin23 --with-sysroot=/Library/Developer/CommandLineTools/SDKs/MacOSX14.sdk Thread model: posix Supported LTO compression algorithms: zlib zstd gcc version 13.2.0 (Homebrew GCC 13.2.0)

A quick test verifies the behavior that we are seeing:

% cat main.f90 
      program TEST1

      call test()
      call test()

      end

% cat test.f90 
      SUBROUTINE test()

      integer ::  ios = 0

      print*,'ios: ', ios

      ios = -1

      return
      end subroutine test

% ./a.out       
 ios:            0
 ios:           -1

[joshsturtevant]

@drakest123 just a +1 that I have also noticed a similar issue in our Clear Creek modeling, where the initialization is fine for the first catchment, but not subsequent ones, e.g.:

NGen Framework 0.1.0
Building Nexus collection
Building Catchment collection
 Reading namelist
  -- simulating basin wb-1561662           with            1  snowbands
 Reading Snow17 parameters
 Reading namelist
  -- simulating basin wb-1561693           with            1  snowbands
 Reading Snow17 parameters
 Read            0  SNOW17 params, but need 26.  Quitting.

[drakest123]

An inventory of INTEGER, REAL and CHARACTER definitions for the NGEN version of the snow17 codebase indicates that, for variables that are not parameters, initialization during declaration occurs only in two files in the share/ directory:

dateTimeUtilsModule.f90:    integer :: y = 4716, j = 1401, m = 2, n = 12, r = 4, p = 1461
dateTimeUtilsModule.f90:    integer :: v = 3, u = 5, s = 153, w = 2, b = 274277, c = - 38
ioModule.f90:    integer                    :: ios=0   ! specify i4b with nrtype?
ioModule.f90:    integer        :: ierr=0   ! error code returned by open(iostat = ierr)
ioModule.f90:    integer                                                     :: nh, ierr=0
ioModule.f90:    integer                      :: ios = 0

[andywood]

digging some more, in 'modern' fortran (post F90) the
integer :: ierr=0
equates to
integer,save :: ierr=0
which means its first initialization is saved but subsequent ones are not. definitely worth going through to upgrade these statements. It's possible a compiler flag like -std=legacy would avoid that, but easy enough to make the change. there's probably no harm in using this for constants (e.g., real :: g=9.80665) but not for variables that will change.

[drakest123]

In the example test.f90 above I tried:
integer,save :: ios=0
but the result remains:

 ios:            0
 ios:           -1

[andywood]

sorry, I probably wasn't clear -- that's the default behavior if you leave out ',save' -- (which is causing the problem).
ok to update the codes and close this issue, now that we understand it?

[joshsturtevant]

Keith's suggested change fixed it for me, i.e., changing line 27 to:

integer :: ios

and then adding the assignment below the local variables:

ios = 0