fix crash when debugging by weiyuan-jiang · Pull Request #173 · GEOS-ESM/GEOSldas_GridComp

weiyuan-jiang · 2026-05-10T01:40:48Z

This PR fixes the run in debug mode. It is 0-diff. It it goes with this PR in NCEP_Shared, GEOSldas can run on debug mode with land assimilation

Related PRs:
GEOS-ESM/NCEP_Shared#37

In debug mode, Intel Fortran raises error 65 (floating invalid) when comparing NaN values (e.g. NaN >= 0.). Input scaling parameter files use NaN as a no-data indicator, which causes comparisons like sclprm_std_obs >= 0. to raise FP exceptions before returning false. Fix: Add NaN guards using the standard Fortran idiom (val == val is .false. for NaN) before the existing range checks at all 3 guard sites in clsm_ensupd_read_obs.F90 (~line 9755, ~9989, ~10384).

When N_files=1, lon_min_vec(2) and lon_max_vec(2) remain initialized to MAPL_UNDEF (1.0e15). Whole-array operations like: start_ind = (lon_min_vec - CMG_ll_lon) / CMG_dlon produce ~2e16 which overflows a 32-bit integer, causing forrtl: error (65): floating invalid in debug mode. Fix: Restrict array operations to (1:N_files) elements only: 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(1:N_files) = last_ind(1:N_files) - start_ind(1:N_files) + 1

gmao-rreichle

@weiyuan-jiang, many thanks for the PR. I have a couple of comments/questions below.

gmao-rreichle · 2026-05-11T14:40:16Z

+       ! 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(:))
+


@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.

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

@weiyuan-jiang : Apologies, I had only checked the latest EASEv2_M09 mwRTM parameter file (used in current SMAP ops), and it didn't have no-data-values for the two "integer" fields (MWRTM_VEGCLS and MWRTM_SOILCLS), as I had expected.
But there are indeed -9999s in the old mwrtm parameter file that is still used in the current nightly tests.
(See below for file locations.)
I suspect the -9999s in the old mwrtm file get turned into 1.e15s somewhere along the way from the mwrtm parameter file to here, which is problematic because -9999 works for real and integer but 1.e15 only works for real. This might have been done for the sake of consistency across restart files (mwRTM_param is the restart file for the assim GridComp), which generally use MAPL_UNDEF (1.e15). This was probably well intentioned at the time, but we didn't think it through all the way.

In any case, since mwRTM_param%vegcls and mwRTM_param%soilcls are both type integer, a cleaner fix might be to define a genuine integer no-data-value, something like:

tmpR = VEGCLS(:) where(tmpR > 1.0e10) mwRTM_param(:)%vegcls = nodata_generic_int4

where

nodata_generic_int4 = -2147483647 ! 1 + type minimum for int*4

or

nodata_generic_int4 = -9999 ! somewhat more intuitive as a no-data-value

And instead of "tmpR > 1.0e10", we should probably use "LDAS_is_nodata(tmpR)", except I think we recently ran into a compiler issue with this elemental function...

Thoughts?

Most recent EASEv2_M09 mwRTM parameters (current SMAP ops):
/discover/nobackup/projects/gmao/smap/SMAP_L4/L4_SM/bcs/RTM_params/RTMParam_SMAP_L4SM_v006/EASEv2_M09/mwRTM_param.nc4

Old EASEv2_M09 mwRTM parameters (nightly tests):
/gpfsm/dnb34/mathomp4/SystemTests/runs/LDAS_GLOBAL/assim/CURRENT/output/SMAP_EASEv2_M09_GLOBAL/rc_out/Y2017/M10/CURRENT.ldas_mwRTMparam.20171015_0000z.nc4

To make it simpler, I think we don't need the extra int4 version.

I do not think we can omit a proper integer nodata_int4 value. As implemented now, this works as long as nodata_generic=-9999. If anyone ever changes nodata_generic to 1.e15, the original problem resurfaces (because in, e.g., Line 2817, 1.e15 is then forced into an integer value)

If anyone changes the parameter nodata_generic's value, then he should change all nodata logics. We don't need to worry about that for now.

@weiyuan-jiang : I much prefer making the complications with the integer no-data-values more explicit. If anyone just changes the value of "nodata_generic", the integer handling issue is likely to be overlooked.
I changed the code accordingly. I hope I didn't mess up, please double-check. Thanks!

That looks fine. If anyone change the values of 'nodata_generic", he has to change nodata_tol_generic, LDAS_is_nodata and the line you just added to define nodata_generic_int4.

gmao-rreichle · 2026-05-11T14:41:06Z

+          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.          &


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.

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

gmao-rreichle · 2026-05-11T14:41:38Z

+          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.  &


same as above

gmao-rreichle · 2026-05-11T14:44:01Z

+          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.  &


same as above

github-actions · 2026-05-11T14:49:43Z

This PR is being prevented from merging because you have added one of our blocking labels: Contingent - DNA, Needs Lead Approval, Contingent -- Do Not Approve. You'll need to remove it before this PR can be merged.

…am%soilcls (GEOS_LandAssimGridComp.F90, LDAS_ensdrv_Globals.F90)

gmao-rreichle · 2026-07-06T15:52:05Z

Weiyuan, Thanks for double-checking. "nodata_tol_generic" is computed from "nodata_generic" and a fractional variable ("nodata_tolfrac_generic"?; designed for single-precision), so "nodata_tol_generic" should automatically be adjusted to whatever "nodata_generic" becomes (in "real" number space). Also, the relevant defs are now all in the "*global*F90" file where the LDAS nodata handling is defined, so it's less likely that the problem we're solving here would be overlooked if anyone changes the nodata-handling in the future. I'll merge when I'm back in the office.

________________________________ From: Weiyuan Jiang ***@***.***> Sent: Monday, July 6, 2026 09:45 To: GEOS-ESM/GEOSldas_GridComp ***@***.***> Cc: Reichle, Rolf (GSFC-6101) ***@***.***>; Review requested ***@***.***> Subject: [EXTERNAL] [BULK] Re: [GEOS-ESM/GEOSldas_GridComp] fix crash when debugging (PR #173) CAUTION: This email originated from outside of NASA. Please take care when clicking links or opening attachments. Use the "Report Message" button to report suspicious messages to the NASA SOC. @weiyuan-jiang commented on this pull request.

________________________________ In GEOSlandassim_GridComp/GEOS_LandAssimGridComp.F90<#173 (comment)>:

+ ! 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(:)) + That looks fine. If anyone change the values of 'nodata_generic", he has to change nodata_tol_generic, LDAS_is_nodata and the line you just added to define nodata_generic_int4. — Reply to this email directly, view it on GitHub<#173?email_source=notifications&email_token=ANDGHMYVUPNEHM2V5IE5Y5T5DOUQFA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINRTGY2DGMBWGU2KM4TFMFZW63VQOJSXM2LFO5PXEZLROVSXG5DFMSSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#discussion_r3529352985>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ANDGHMYMF7ME4FZ5SKGTQID5DOUQFAVCNFSNUABFKJSXA33TNF2G64TZHM3TONZUGE3TSMBRHNEXG43VMU5TINBRGQZTGMJYGE32C5QC>. Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/ANDGHM62Q2RZOWNHB564MZT5DOUQFA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINRTGY2DGMBWGU2KM4TFMFZW63VQOJSXM2LFO5PXEZLROVSXG5DFMSSWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android<https://github.com/notifications/mobile/android/ANDGHMYNNHVAHCEWURKEUHT5DOUQFA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINRTGY2DGMBWGU2KM4TFMFZW63VQOJSXM2LFO5PXEZLROVSXG5DFMSSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today! You are receiving this because your review was requested.Message ID: ***@***.***>

weiyuan-jiang added 3 commits May 8, 2026 16:44

work around compiler bug

554f786

weiyuan-jiang requested a review from a team as a code owner May 10, 2026 01:40

weiyuan-jiang added the 0-diff label May 10, 2026

change log

f97633d

weiyuan-jiang changed the title ~~Feature/wjiang/fix debug~~ fix crash when debugging May 11, 2026

gmao-rreichle reviewed May 11, 2026

View reviewed changes

gmao-rreichle added bug fix Contingent -- Do Not Approve labels May 11, 2026

gmao-rreichle marked this pull request as draft May 11, 2026 14:49

gmao-rreichle and others added 6 commits May 20, 2026 17:04

Merge branch 'develop' into feature/wjiang/fix_debug

16a4280

change nodata

e4a3960

updated CHANGELOG.md

49bef08

improved vertical alignment (clsm_ensupd_enkf_update.F90)

de9da6e

addded comments; improved vertical alignment (clsm_ensupd_read_obs.F90)

3597af3

revised integer no-data-handling for mwrtm_param%vegcls and mwrtm_par…

9203d9e

…am%soilcls (GEOS_LandAssimGridComp.F90, LDAS_ensdrv_Globals.F90)

Uh oh!

Conversation

weiyuan-jiang commented May 10, 2026 • edited by gmao-rreichle Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gmao-rreichle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weiyuan-jiang May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 11, 2026

Uh oh!

gmao-rreichle commented Jul 6, 2026 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

weiyuan-jiang commented May 10, 2026 •

edited by gmao-rreichle

Loading

weiyuan-jiang May 11, 2026 •

edited

Loading