hotfix - safeguard invalid scalar indices to prevent segmentation faults during LBC/IC generation - #263
Conversation
…g LBC/IC genereation
e4a5d7c to
e66493a
Compare
|
@guoqing-noaa, I was able to recreate this issue. I think that the config variables |
| if (associated(index_dust_coarse ) ) sorted_arrQ13(:,:) = -999.0 | ||
| scalars(index_nwfa,:,iCell) = 0._RKIND | ||
| scalars(index_nifa,:,iCell) = 0._RKIND | ||
| if (index_nwfa > 0) scalars(index_nwfa,:,iCell) = 0._RKIND |
There was a problem hiding this comment.
I wouldn't touch anything that is not in the init_atm_case_lbc subroutine for now.
| if (index_nwfa > 0) scalars(index_nwfa,k,iCell) = vertical_interp(target_z, nfglevels_actual-1, & | ||
| sorted_arrQ9(:,1:nfglevels_actual-1), order=1, extrap=0) | ||
| scalars(index_nifa,k,iCell) = vertical_interp(target_z, nfglevels_actual-1, & | ||
| if (index_nifa > 0) scalars(index_nifa,k,iCell) = vertical_interp(target_z, nfglevels_actual-1, & |
There was a problem hiding this comment.
I wouldn't touch anything that is not in the init_atm_case_lbc subroutine for now.
| if (associated(index_smoke_fine )) sorted_arrQ11(:,:) = -999.0 | ||
| if (associated(index_dust_fine )) sorted_arrQ12(:,:) = -999.0 | ||
| if (associated(index_dust_coarse )) sorted_arrQ13(:,:) = -999.0 | ||
| scalars(index_nwfa,:,iCell) = 0._RKIND |
There was a problem hiding this comment.
see comment on suggested changes using the config variable, which is safer
There was a problem hiding this comment.
@AndersJensen-NOAA I understand your preference to use config_lbc_hydrometeors_gfs/config_lbc_hydrometeors_rrfs to do the safe guard. However, why using the config_lbc options is safer? Could you elaborate more?
To my understanding, if (index_qc > 0) scalars(index_qc,:,iCell) = 0._RKIND is a more generic safe guard. Even one changes what variables exist for each different config_lbc options, the if (index_qc > 0) logic is still valid and don't need modifications.
|
@guoqing-noaa if you're able to address Anders' comments before next Wednesday (7/29), we should be able to get this merged by then. |
|
@AndersJensen-NOAA Thanks for the review and disucssion. I would think Similar strategy has been used in other places in MPAS-Model/src/core_init_atmosphere/mpas_init_atm_cases.F Lines 8858 to 8867 in 02295fe However, if you would like to use the config_lbc option, I am fine to change accordingly to retrieve the config_lbc options inside the subroutine and use them to do the safeguard. For I will restore to leave them untouched. But we know it poses a potential issue there since the availability of nifa and nwfa depends on a certain package and it may give us a similar "silent" segmentation fault in some situations. |
|
Update: @AndersJensen-NOAA I think I got your concern. In some systems, dereferencing a disassociated pointer is undefined behavior, so On the other hand, we may want to use a more robust safe guard that handles any package combination that might enable/disable the field. I would suggest we adopt the method used by chemistry code (see below examples) to be more robust and future-proof. |
@keenan and I got the same lbc task crash on Hera recently. All lbc files were generated correctly but the task failed due to
segmentation fault occurred:It looks similar to the behavior we met before when running the model where the executable just could not terminate correctly.
I tried a debug build and identified that the crash happened at line 8577 of
mpas_init_atm_cases.F:and lines 8577-8581 read as follows:
So it suggested a
-1ofindex_nccaused the crash.In
src/core_init_atmosphere/Registry.xml, we can find that whenlbc_hydrometeors_gfsis true whilelbc_hydrometeors_rrfsis false, we only havelbc_qv, lbc_qc, lbc_qr, lbc_qi, lbc_qs, lbc_qg, do not havelbc_nc/ni,..... Hence we will of course get-1for their indexes (index_nc, ect).This PR provides a hotfix by checking
if (index_nc > 0)before settingscalars(index_nc,:,iCell) = 0._RKIND.Similar safeguards are applied to IC generation where
index_nwfaandindex_nifaare only available for specific packages.I don't have a full answer to why this only crashes on Hera but not on other machines.
Different machines have different memory/heap layout, the out-of-bounds address may still be within a valid memory page, so no SIGSEGV occurs and the program finishes "successfully".
Mandatory Questions
Reviews