Skip to content

fix crash when debugging #173

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

Draft
weiyuan-jiang wants to merge 4 commits into
base: develop
Choose a base branch
from
Draft

fix crash when debugging #173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
36d9f60
Guard NaN comparisons in sclprm scaling conditions
weiyuan-jiang May 8, 2026
3330273
Fix integer overflow from MAPL_UNDEF in read_obs_MODIS_SCF
weiyuan-jiang May 8, 2026
554f786
work around compiler bug
weiyuan-jiang May 9, 2026
f97633d
change log
weiyuan-jiang May 10, 2026
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Fixed some crashes in debug mode
- Workaround compiler bug where(elemental) by extra mask
- Fixed string matching for EASE tile file to accommodate new "EASE*-Pfafstetter" tile file for runoff routing purposes.
- Fixed GEOSlandpert build when MKL is unavailable by enabling MKL-specific code paths only when MKL is detected.
- Fixed NAG Fortran compiler issues.
Expand Down
13 changes: 10 additions & 3 deletions GEOSlandassim_GridComp/GEOS_LandAssimGridComp.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2758,7 +2758,7 @@ subroutine get_mwrtm_param(MAPL, clock, N_catl, INTERNAL, rc)

integer :: N_catl_tmp, n, mpierr, status
logical :: mwp_nodata, all_nodata_l

real, allocatable :: tmpR(:)

if(.not. allocated(mwRTM_param)) then

Expand Down Expand Up @@ -2806,8 +2806,15 @@ subroutine get_mwrtm_param(MAPL, clock, N_catl, INTERNAL, rc)

allocate(mwRTM_param(N_catl))
mwRTM_param(:)%sand = SAND(:)
mwRTM_param(:)%vegcls = nint(VEGCLS(:))
mwRTM_param(:)%soilcls = nint(SOILCLS(:))
! when in debug mode, nint(VEGCLS) with 1.0e15 may crash
allocate(tmpR(N_catl))
tmpR = VEGCLS(:)
where(tmpR > 1.0e10) tmpR = nodata_generic
mwRTM_param(:)%vegcls = nint(tmpR(:))
tmpR = SOILCLS(:)
where(tmpR > 1.0e10) tmpR = nodata_generic
mwRTM_param(:)%soilcls = nint(tmpR(:))

Comment on lines +2809 to +2817
Copy link
Copy Markdown
Collaborator

@gmao-rreichle gmao-rreichle May 11, 2026

Choose a reason for hiding this comment

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

@weiyuan-jiang : I'm not sure if we need this, and if we do, I'm not sure it's the best fix. First, I'm pretty confident that vegcls and soilcls here should never be no-data. So a better way of addressing this might be to check for (native) no-data-values first and stop if any are encountered.
If I'm wrong and no-data-values for the "integer" fields could happen, we might need to figure out a more robust solution. The current solution depends on "nodata_generic" being -9999. I was hoping that at some point in the future we can use MAPL_UNDEF instead of LDAS having its own nodata-value. But if nodata_generic turns into 1e15, the currently implemented solution won't work, I think.

Copy link
Copy Markdown
Contributor Author

@weiyuan-jiang weiyuan-jiang May 11, 2026

Choose a reason for hiding this comment

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

When I debugged, I printed the values and I did see 1.0e15. When it is converted to int, it becomes the most negative integer. Here our nondata_generic is -9999.0

mwRTM_param(:)%clay = CLAY(:)
mwRTM_param(:)%poros = mw_POROS(:)
mwRTM_param(:)%wang_wt = WANGWT(:)
Expand Down
52 changes: 19 additions & 33 deletions GEOSlandassim_GridComp/clsm_ensupd_enkf_update.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1807,7 +1807,7 @@ subroutine write_ObsFcstAna_nc4(fname, exp_id, N_obsf, Observations_f, &
character(len=128) :: created_by
integer :: user_len, user_status
integer :: i

logical, allocatable :: mask(:)
integer, dimension(N_obsf) :: tmpvecint
real, dimension(N_obsf) :: tmpvecreal

Expand Down Expand Up @@ -1944,50 +1944,36 @@ subroutine write_ObsFcstAna_nc4(fname, exp_id, N_obsf, Observations_f, &

! for assim flag, convert logical to integer
tmpvecint = 0
where (Observations_f(1:N_obsf)%assim)
tmpvecint = 1
end where
call nc4_check( nf90_put_var(ncid, assim_flag_varid, tmpvecint) )

where (Observations_f(1:N_obsf)%assim) tmpvecint = 1; call nc4_check( nf90_put_var(ncid, assim_flag_varid, tmpvecint) )

! for data fields, replace LDAS no-data-value with MAPL_UNDEF for consistency with MAPL HISTORY output
tmpvecreal = Observations_f(1:N_obsf)%obs
do i=1,N_obsf
if (LDAS_is_nodata(tmpvecreal(i))) tmpvecreal(i) = MAPL_UNDEF
end do
call nc4_check( nf90_put_var(ncid, obs_varid, tmpvecreal) )
mask = LDAS_is_nodata(tmpvecreal)
where (mask) tmpvecreal=MAPL_UNDEF; call nc4_check( nf90_put_var(ncid, obs_varid, tmpvecreal))

tmpvecreal = Observations_f(1:N_obsf)%obsvar
do i=1,N_obsf
if (LDAS_is_nodata(tmpvecreal(i))) tmpvecreal(i) = MAPL_UNDEF
end do
call nc4_check( nf90_put_var(ncid, obsvar_varid, tmpvecreal) )
mask = LDAS_is_nodata(tmpvecreal)
where (mask) tmpvecreal=MAPL_UNDEF; call nc4_check( nf90_put_var(ncid, obsvar_varid, tmpvecreal))

tmpvecreal = Observations_f(1:N_obsf)%fcst
do i=1,N_obsf
if (LDAS_is_nodata(tmpvecreal(i))) tmpvecreal(i) = MAPL_UNDEF
end do
call nc4_check( nf90_put_var(ncid, fcst_varid, tmpvecreal) )

mask = LDAS_is_nodata(tmpvecreal)
where (mask) tmpvecreal=MAPL_UNDEF; call nc4_check( nf90_put_var(ncid, fcst_varid, tmpvecreal))

tmpvecreal = Observations_f(1:N_obsf)%fcstvar
do i=1,N_obsf
if (LDAS_is_nodata(tmpvecreal(i))) tmpvecreal(i) = MAPL_UNDEF
end do
call nc4_check( nf90_put_var(ncid, fcstvar_varid, tmpvecreal) )

mask = LDAS_is_nodata(tmpvecreal)
where (mask) tmpvecreal=MAPL_UNDEF; call nc4_check( nf90_put_var(ncid, fcstvar_varid, tmpvecreal))

tmpvecreal = Observations_f(1:N_obsf)%ana
do i=1,N_obsf
if (LDAS_is_nodata(tmpvecreal(i))) tmpvecreal(i) = MAPL_UNDEF
end do
call nc4_check( nf90_put_var(ncid, ana_varid, tmpvecreal) )
mask = LDAS_is_nodata(tmpvecreal)
where (mask) tmpvecreal=MAPL_UNDEF; call nc4_check( nf90_put_var(ncid, ana_varid, tmpvecreal))

tmpvecreal = Observations_f(1:N_obsf)%anavar
do i=1,N_obsf
if (LDAS_is_nodata(tmpvecreal(i))) tmpvecreal(i) = MAPL_UNDEF
end do
call nc4_check( nf90_put_var(ncid, anavar_varid, tmpvecreal) )

mask = LDAS_is_nodata(tmpvecreal)
where (mask) tmpvecreal=MAPL_UNDEF; call nc4_check( nf90_put_var(ncid, anavar_varid, tmpvecreal))

end if


if (N_obs_param > 0) then

allocate(species_assim_int( N_obs_param))
Expand Down
24 changes: 18 additions & 6 deletions GEOSlandassim_GridComp/clsm_ensupd_read_obs.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5979,10 +5979,10 @@ subroutine read_obs_MODIS_SCF( &

N_lat = last_ind(1) - start_ind(1) + 1

start_ind = (lon_min_vec - CMG_ll_lon)/CMG_dlon
last_ind = (lon_max_vec - CMG_ll_lon)/CMG_dlon
start_ind(1:N_files) = (lon_min_vec(1:N_files) - CMG_ll_lon)/CMG_dlon
last_ind(1:N_files) = (lon_max_vec(1:N_files) - CMG_ll_lon)/CMG_dlon

N_lon_vec = last_ind - start_ind + 1
N_lon_vec(1:N_files) = last_ind(1:N_files) - start_ind(1:N_files) + 1

N_lon = sum( N_lon_vec(1:N_files) )

Expand Down Expand Up @@ -9752,7 +9752,11 @@ subroutine scale_obs_tskin_zscore( N_catd, tile_coord, &
! check for no-data-values in observation and fit parameters
! (any negative number could be no-data-value for observations)

if ( sclprm_mean_obs(ind)>0. .and. &
if ( sclprm_mean_obs(ind)==sclprm_mean_obs(ind) .and. &
sclprm_mean_mod(ind)==sclprm_mean_mod(ind) .and. &
sclprm_std_obs(ind) ==sclprm_std_obs(ind) .and. &
sclprm_std_mod(ind) ==sclprm_std_mod(ind) .and. &
Comment on lines +9755 to +9758
Copy link
Copy Markdown
Collaborator

@gmao-rreichle gmao-rreichle May 11, 2026

Choose a reason for hiding this comment

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

Why do we need these four lines, which should always be true? Not sure where they're coming from and what they might be meant to do.
Or do they evaluate to "false" if a value is NaN? If this is the intent, we should at least add a comment to clarify. Although I don't think the reader for the "sclprm_*" values would produce NaNs.
Alternatively, perhaps check more explicitly for nodata values?
I guess all of this depends on the nodata-value here being -9999. If the nodata-value is 1e15, it wouldn't be identified by any of the if-conditions.

Copy link
Copy Markdown
Contributor Author

@weiyuan-jiang weiyuan-jiang May 11, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I think that comparison is for Nan. If Nan == Nan is always .false. . In debugging mode, comparing Nan may lead to crash

sclprm_mean_obs(ind)>0. .and. &
sclprm_mean_mod(ind)>0. .and. &
sclprm_std_obs(ind)>=0. .and. &
sclprm_std_mod(ind)>=0. ) then
Expand Down Expand Up @@ -9986,7 +9990,11 @@ subroutine scale_obs_sfmc_zscore( N_catd, tile_coord, &
! Check for no-data-values in observation and fit parameters
! (any negative number could be no-data-value for observations)

if ( sclprm_mean_obs(j_ind, i_ind)>0. .and. &
if ( sclprm_mean_obs(j_ind, i_ind)==sclprm_mean_obs(j_ind, i_ind) .and. &
sclprm_mean_mod(j_ind, i_ind)==sclprm_mean_mod(j_ind, i_ind) .and. &
sclprm_std_obs(j_ind, i_ind) ==sclprm_std_obs(j_ind, i_ind) .and. &
sclprm_std_mod(j_ind, i_ind) ==sclprm_std_mod(j_ind, i_ind) .and. &
Comment on lines +9993 to +9996
Copy link
Copy Markdown
Collaborator

@gmao-rreichle gmao-rreichle May 11, 2026

Choose a reason for hiding this comment

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

same as above

sclprm_mean_obs(j_ind, i_ind)>0. .and. &
sclprm_mean_mod(j_ind, i_ind)>0. .and. &
sclprm_std_obs(j_ind, i_ind)>=0. .and. &
sclprm_std_mod(j_ind, i_ind)>=0. ) then
Expand Down Expand Up @@ -10381,7 +10389,11 @@ subroutine scale_obs_Tb_zscore( N_catd, tile_coord, date_time, this_obs_param,
! check for no-data-values in observation and fit parameters
! (any negative number could be no-data-value for observations)

if ( sclprm_mean_obs(ind)>0. .and. &
if ( sclprm_mean_obs(ind)==sclprm_mean_obs(ind) .and. &
sclprm_mean_mod(ind)==sclprm_mean_mod(ind) .and. &
sclprm_std_obs( ind)==sclprm_std_obs( ind) .and. &
sclprm_std_mod( ind)==sclprm_std_mod( ind) .and. &
Comment on lines +10392 to +10395
Copy link
Copy Markdown
Collaborator

@gmao-rreichle gmao-rreichle May 11, 2026

Choose a reason for hiding this comment

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

same as above

sclprm_mean_obs(ind)>0. .and. &
sclprm_mean_mod(ind)>0. .and. &
sclprm_std_obs( ind)>0. .and. &
sclprm_std_mod( ind)>0. ) then
Expand Down
4 changes: 2 additions & 2 deletions LDAS_Shared/LDAS_ensdrv_Globals.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ module LDAS_ensdrv_Globals
real, parameter :: nodata_generic = -9999.
real, parameter :: nodata_tolfrac_generic = 1.e-4

real :: nodata_tol_generic = abs(nodata_generic*nodata_tolfrac_generic)
real :: MAPL_UNDEF_tol_generic = abs(MAPL_UNDEF *nodata_tolfrac_generic)
real, parameter :: nodata_tol_generic = abs(nodata_generic*nodata_tolfrac_generic)
real, parameter :: MAPL_UNDEF_tol_generic = abs(MAPL_UNDEF *nodata_tolfrac_generic)

! ----------------------------------------------------------------
!
Expand Down
Loading