Skip to content

Replace stop with return and provide and bmi_failure flags for EnergyModule error #108

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SnowHydrology wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Replace stop with return and provide and bmi_failure flags for EnergyModule error #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f939e07
return and bmi_failure flags for energymodule error
SnowHydrology Mar 27, 2024
be6a639
Changed STOP to RETURN with signature changes up call stack
drakest123 May 13, 2024
0f4dc8c
Replaced STOP and sys_abort() with RETURN up call stack
drakest123 May 14, 2024
989ba81
Commented out unused SUBROUTINE parse() to eliminate stop statement
drakest123 May 14, 2024
4c8a086
initial changes removing error_flag from call signatures
drakest123 Jun 11, 2024
5fcfeed
Introduce ErrorType to return error to calling program
drakest123 Jun 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions bmi/bmi_noahowp.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,14 @@ function noahowp_initialize(this, config_file) result (bmi_status)
character (len=*), intent(in) :: config_file
integer :: bmi_status

bmi_status = BMI_SUCCESS
if (len(config_file) > 0) then
call initialize_from_file(this%model, config_file)
else
!call initialize_from_defaults(this%model)
end if
bmi_status = BMI_SUCCESS
bmi_status = this%model%error%error_flag

end function noahowp_initialize

! BMI finalizer.
Expand Down Expand Up @@ -267,9 +269,11 @@ end function noahowp_time_units
function noahowp_update(this) result (bmi_status)
class (bmi_noahowp), intent(inout) :: this
integer :: bmi_status
bmi_status = BMI_SUCCESS

call advance_in_time(this%model)
bmi_status = BMI_SUCCESS
bmi_status = this%model%error%error_flag

@PhilMiller PhilMiller Aug 21, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to do the same to noahowp_update_until just below

end function noahowp_update

! Advance the model until the given time.
Expand Down
47 changes: 33 additions & 14 deletions driver/AsciiReadModule.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
module AsciiReadModule

use UtilitiesModule
use ErrorCheckModule

implicit none
character(len=*), PARAMETER :: moduleName='AsciiReadModule'
private :: moduleName

contains

Expand All @@ -11,6 +14,7 @@ subroutine open_forcing_file(filename)
implicit none

character*256, intent(in) :: filename
character(len=*), PARAMETER :: subroutineName = 'open_forcing_file'

!---------------------------------------------------------------------
! local variables
Expand All @@ -22,17 +26,19 @@ subroutine open_forcing_file(filename)
! Check if the specified file exists
inquire(file = trim(filename), exist = lexist)
if (.not. lexist) then
write(*,'(/," ***** Problem *****")')
write(*,'(" ***** File ''", A, "'' does not exist.")') trim(filename)
write(*,'(" ***** Check the forcing file specified as a command-line argument",/)')
stop ": ERROR EXIT"
error_flag = NOM_FAILURE
write(error_string,'(A,A,A)') 'AsciiReadModule'//' - '//subroutineName//'(): File: ',trim(filename), ' does not exist. Check the forcing file specified as a command-line argument.'
call log_message(error_flag, error_string)

@GreyREvenson GreyREvenson May 22, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment regarding writing to a possible public character variable in ErrorCheckModule instead of calling log_message to print right away.

@GreyREvenson GreyREvenson May 22, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, seems like kind of a bummer that we'd have to manually record where in the program the error occurs via the error message. My understanding is that this is because we're going to use return statements instead of stop statements in which case the program won't record any locational information (i.e., where in the code the error happens). Is there any way to automatically record locational information to pass along side the error code and message?

@PhilMiller PhilMiller May 23, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this code is being run through the C pre-processor (as use of #ifdef NGEN_ACTIVE suggests), then we could define a macro along the lines of

#define NOAH_OWP_MODULAR_REPORT_ERROR(error_string) \
    error_flag = NOM_FAILURE \
    call log_error_message(error_flag, error_string, __FILE__, __LINE__)

Those __FILE__ and __LINE__ macros would capture the location of the error report, though not the function name

I'm a touch concerned that these files might not get pre-processed as expected, since that usually seems to go with a .F90 (upper-case) file extension. As I understand it, though, that's just a convention, and the build scripts could do whatever. The shift to a cmake-driven build system may change that, though. That pre-processing behavior based on .f90 vs .F90 is documented functionality of GNU Fortran. For .f90 files, it can also be enabled by command line flags from the build scripts.

@drakest123 drakest123 May 24, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered the preprocessor approach that @PhilMiller discusses. Besides the issues with this approach that Phil brings up, I don't think that using preprocessor directives for delineating error sources would be appropriate. Currently, the program has sparse reliance on preprocessor directives and they are used for major functional conditions such as whether the code is run standalone or as a module. These few preprocessor directives could easily be removed so the reliance on them is easy to decouple.

One possible intermediate improvement would be to define a string that contains the subroutine name and another string with module scope that contains the module name. Then, the error string would refer to those variables instead of them being hardwired. Given that LINE would unambiguously identify the line where the error is reported, unique error messages alternatively identify error sources although the onus is more on the programmer.

return
endif

! Open the forcing file
open(10, file = trim(filename), form = 'formatted', action = 'read', iostat = ierr)
if (ierr /= 0) then
write(*,'("Problem opening file ''", A, "''")') trim(filename)
stop ": ERROR EXIT"
error_flag = NOM_FAILURE
write(error_string,'(A,A,A)') 'AsciiReadModule'//' - '//subroutineName//'(): Problem opening file: ',trim(filename)
call log_message(error_flag, error_string)
return
endif

end subroutine open_forcing_file
Expand Down Expand Up @@ -110,6 +116,7 @@ subroutine read_forcing_text(iunit, nowdate, forcing_timestep, &
real, parameter :: eps = 0.622

character(len=1024) :: string
character(len=*), PARAMETER :: subroutineName = 'read_forcing_text'

! Flag to tell us whether this is the first time this subroutine is called, in which case
! we need to seek forward to the data.
Expand Down Expand Up @@ -166,10 +173,11 @@ subroutine read_forcing_text(iunit, nowdate, forcing_timestep, &

return
endif
if (ierr /= 0) then
write(*,'("Error reading from data file.")')
ierr = 2
return
if (ierr /= 0) then
error_flag = NOM_FAILURE
write(error_string,'(A)') 'AsciiReadModule'//' - '//subroutineName//'(): Error reading from data file.'
call log_message(error_flag, error_string)
return
endif
write(readdate,'(I4.4,4I2.2)') year, month, day, hour, minute

Expand All @@ -185,7 +193,10 @@ subroutine read_forcing_text(iunit, nowdate, forcing_timestep, &
before = fdata ( readdate, read_windspeed, read_winddir, read_temperature, read_humidity, read_pressure, read_swrad, read_lwrad, read_rain )
cycle READLOOP
else
stop "Logic problem"
error_flag = NOM_FAILURE
write(error_string,'(A)') 'AsciiReadModule'//' - '//subroutineName//'(): Logic problem.'
call log_message(error_flag, error_string)
return
endif
enddo READLOOP

Expand Down Expand Up @@ -218,6 +229,9 @@ subroutine read_forcing_text(iunit, nowdate, forcing_timestep, &

call geth_idts(nowdate, before%readdate, idts)
call geth_idts(after%readdate, before%readdate, idts2)
if (error_flag == NOM_FAILURE) then
return
endif

if (idts2*60 /= forcing_timestep) then
print*, 'forcing_timestep = ', forcing_timestep
Expand All @@ -226,7 +240,10 @@ subroutine read_forcing_text(iunit, nowdate, forcing_timestep, &
print*, 'idts = ', idts
print*,' after%readdate = ', after%readdate
print*, 'idts2 = ', idts2
stop "IDTS PROBLEM"
error_flag = NOM_FAILURE
write(error_string,'(A)') 'AsciiReadModule'//' - '//subroutineName//'(): IDTS PROBLEM.'
call log_message(error_flag, error_string)
return
endif

fraction = real(idts2-idts)/real(idts2)
Expand All @@ -250,8 +267,10 @@ subroutine read_forcing_text(iunit, nowdate, forcing_timestep, &
rhf = rhf * 1.E-2

else
print*, 'nowdate = "'//nowdate//'"'
stop "Problem in the logic of read_forcing_text."
error_flag = NOM_FAILURE
write(error_string,'(A,A,A)') 'AsciiReadModule'//' - '//subroutineName//'(): date: ', nowdate, '. Problem in the logic of read_forcing_text.'
call log_message(error_flag, error_string)
return
endif

! Below commented out KSJ 2021-06-09
Expand Down
16 changes: 16 additions & 0 deletions driver/NoahModularDriver.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ program model_driver
print*, "Initializing..."
call get_command_argument(1, arg)
status = m%initialize(arg)
if (status == BMI_FAILURE) then

@PhilMiller PhilMiller Aug 21, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (status == BMI_FAILURE) then
if (status /= BMI_SUCCESS) then

Being kind of pedantic against introduction of other non-success values

#ifdef NGEN_ACTIVE
return status ! if NGEN
#else
print*, "Stopping program."
stop
#endif
end if

!---------------------------------------------------------------------
! Run the model with BMI
Expand All @@ -43,6 +51,14 @@ program model_driver
print*, "Running..."
do while (current_time < end_time)
status = m%update() ! run the model one time step
if (status == BMI_FAILURE) then
#ifdef NGEN_ACTIVE
return status ! if NGEN
#else
print*, "Stopping program."
stop
#endif
end if
status = m%get_current_time(current_time) ! update current_time
end do

Expand Down
20 changes: 20 additions & 0 deletions err_flag_info.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
./ParametersRead.f90: SUBROUTINE read_rad_parameters(param_dir, noahowp_table, error_flag)
./ParametersRead.f90: subroutine read_global_parameters(param_dir, noahowp_table, error_flag)
./ParametersRead.f90: SUBROUTINE read_crop_parameters(param_dir, noahowp_table, error_flag)
./ParametersRead.f90: SUBROUTINE read_irrigation_parameters(param_dir, noahowp_table, error_flag)
./ParametersRead.f90: SUBROUTINE read_tiledrain_parameters(param_dir, noahowp_table, error_flag)
./ParametersRead.f90: SUBROUTINE read_optional_parameters(param_dir, noahowp_table, error_flag)
./RunModule.f90: call namelist%ReadNamelist(config_filename,domain%error_flag)
./RunModule.f90: call parameters%paramRead(namelist, model%domain%error_flag)
./RunModule.f90: call get_utime_list (domain%start_datetime, domain%end_datetime, domain%dt, domain%sim_datetimes, domain%error_flag) ! makes unix-time list for desired records (end-of-timestep)
./RunModule.f90: call open_forcing_file(namelist%forcing_filename, model%domain%error_flag)
./NamelistRead.f90: subroutine ReadNamelist(this, namelist_file, error_flag)
./UtilitiesModule.f90: subroutine geth_idts (newdate, olddate, idt, error_flag)
./UtilitiesModule.f90: call geth_idts(nowdate(1:10), nowdate(1:4)//"-01-01", iday, error_flag)
./DateTimeUtilsModule.f90: subroutine get_utime_list (start_datetime, end_datetime, dt, times, error_flag)
./ParametersType.f90: subroutine paramRead(this, namelist, error_flag)
./ParametersType.f90: call read_soil_parameters(namelist%parameter_dir, namelist%soil_table, namelist%general_table, namelist%soil_class_name, error_flag)
./ParametersType.f90: call read_veg_parameters(namelist%parameter_dir, namelist%noahowp_table, namelist%veg_class_name, error_flag)
./ParametersType.f90: call read_rad_parameters(namelist%parameter_dir, namelist%noahowp_table, error_flag)
./ParametersType.f90: call read_global_parameters(namelist%parameter_dir, namelist%noahowp_table, error_flag)
./EnergyModule.f90: call log_message(domain%error_flag, trim(error_string))
1 change: 1 addition & 0 deletions run/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ include ../user_build_options

OBJS = \
../src/ErrorCheckModule.o \
../src/ErrorType.o \
../src/NamelistRead.o \
../src/ParametersRead.o \
../src/LevelsType.o \
Expand Down
82 changes: 45 additions & 37 deletions src/DateTimeUtilsModule.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@

module DateTimeUtilsModule

use ErrorCheckModule

implicit none
character(len=*), PARAMETER :: moduleName='DateTimeUtilsModule'
private :: moduleName
public

integer, parameter :: kr4 = selected_real_kind (6, 37)! single precision real
Expand Down Expand Up @@ -61,42 +65,42 @@ module DateTimeUtilsModule

!**********************************************************************

subroutine parse (str, delims, args, nargs)
! subroutine parse (str, delims, args, nargs)

! Parses the string 'str' into arguments args(1), ..., args(nargs) based on
! the delimiters contained in the string 'delims'. Preceding a delimiter in
! 'str' by a backslash (\) makes this particular instance not a delimiter.
! The integer output variable nargs contains the number of arguments found.

character (len=*) :: str, delims
character (len=len_trim(str)) :: strsav
character (len=*), dimension (:) :: args
integer :: i, k, na, nargs, lenstr

strsav = str
call compact (str)
na = size (args)
do i = 1, na
args (i) = ' '
end do
nargs = 0
lenstr = len_trim (str)
if (lenstr == 0) return
k = 0

do
if (len_trim(str) == 0) exit
nargs = nargs + 1
if(nargs .gt. size(args)) then
print *,'Number of predictors larger than expected, check nPredict'
stop
end if
call split (str, delims, args(nargs))
call removebksl (args(nargs))
end do
str = strsav

end subroutine parse
! character (len=*) :: str, delims
! character (len=len_trim(str)) :: strsav
! character (len=*), dimension (:) :: args
! integer :: i, k, na, nargs, lenstr

! strsav = str
! call compact (str)
! na = size (args)
! do i = 1, na
! args (i) = ' '
! end do
! nargs = 0
! lenstr = len_trim (str)
! if (lenstr == 0) return
! k = 0

! do
! if (len_trim(str) == 0) exit
! nargs = nargs + 1
! if(nargs .gt. size(args)) then
! print *,'Number of predictors larger than expected, check nPredict'
! stop
! end if
! call split (str, delims, args(nargs))
! call removebksl (args(nargs))
! end do
! str = strsav

! end subroutine parse

@PhilMiller PhilMiller Aug 21, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a routine is being deleted, then just delete it, rather than leaving it commented out. Regardless, I don't think that change belongs in this PR.


!**********************************************************************

Expand Down Expand Up @@ -879,14 +883,16 @@ double precision function date_to_unix (date)
character (len=*), intent (in) :: date
double precision :: u_day, i_day, days
integer :: sec, min, hour, day, month, year, error
character(len=*), PARAMETER :: subroutineName = 'date_to_unix'

call parse_date (date, year, month, day, hour, min, sec, error)

if (error /= 0) then
error_flag = NOM_FAILURE
date_to_unix = -9999.99
print*, 'error in date_to_unix -- date, year, month, day, hour, min, sec, error:'
print*, date, year, month, day, hour, min, sec, error
stop !return
write(error_string,'(A,A,A,I4,A,I4,A,I4,A,I4,A,I4,A,I4,A,I4)') moduleName//" - "//subroutineName//"(): date: ",date," year: ",year," month: ",month," day: ",day," hour: ",hour," min: ",min," sec: ",sec, " Error: ",error
call log_message(error_flag, error_string)
return
end if

u_day = julian_date (1, 1, 1970)
Expand Down Expand Up @@ -970,12 +976,14 @@ subroutine get_utime_list (start_datetime, end_datetime, dt, times)
!local
integer :: t, ntimes
real*8 :: utime
character(len=*), PARAMETER :: subroutineName = 'get_utime_list'

if(abs(mod(end_datetime - start_datetime, dt)) > 1e-5) then
print*, 'start and end datetimes are not an even multiple of dt -- check dates in namelist'
print*, 'end_datetime, start_datetime, dt, mod:', end_datetime, start_datetime, dt, mod(end_datetime-start_datetime, dt)
stop
end if
error_flag = NOM_FAILURE
write(error_string,'(A,G8.3,A,G8.3,A,G8.3,A,G8.3)') moduleName//" - "//subroutineName//"(): start and end datetimes are not an even multiple of dt -- check dates in namelist: end_datetime: ",end_datetime," start_datetime: ",start_datetime," dt: ",dt," mod: ", mod(end_datetime-start_datetime, dt)
call log_message(error_flag, error_string)
return
endif

ntimes = int((end_datetime - start_datetime)/dt) + 1
allocate (times(ntimes))
Expand Down
12 changes: 12 additions & 0 deletions src/DomainType.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ module DomainType

use NamelistRead, only: namelist_type
use DateTimeUtilsModule
use ErrorCheckModule

implicit none
save
Expand Down Expand Up @@ -31,6 +32,7 @@ module DomainType
integer :: croptype ! crop type
integer :: isltyp ! soil type
integer :: IST ! surface type 1-soil; 2-lake

real, allocatable, dimension(:) :: zsoil ! depth of layer-bottom from soil surface
real, allocatable, dimension(:) :: dzsnso ! snow/soil layer thickness [m]
real, allocatable, dimension(:) :: zsnso ! depth of snow/soil layer-bottom
Expand Down Expand Up @@ -117,7 +119,17 @@ subroutine InitTransfer(this, namelist)
this%isltyp = namelist%isltyp
this%IST = namelist%sfctyp
this%start_datetime = date_to_unix(namelist%startdate) ! returns seconds-since-1970-01-01
if (this%start_datetime < 0) then
error_flag = NOM_FAILURE
error_string = 'DomainType - InitTransfer(): Invalid start time'
return
endif
this%end_datetime = date_to_unix(namelist%enddate)
if (this%end_datetime < 0) then
error_flag = NOM_FAILURE
error_string = 'DomainType - InitTransfer(): Invalid end time'
return
endif

end subroutine InitTransfer

Expand Down
Loading