You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I was able to fix our valgrind failures by doing a few mods. Namely:
use pack size 1 in our valg build: avoids issues with pow(UNINITED,exponent). The altearnative is to ensure the base is always inited, but there may be quite a few ramifications.
set some defaults for se_XYZ namelist variables in HOMME before reading them, in case they are missing in the input namelist. Usually, they are missing if not used (e.g., no point in setting se_ne for DP runs, since se_ne_x and se_ne_y will be used), but valgrind doesn't know that. @oksanaguba I added you as a reviewer for the handful of lines in namelist mod, just in case.
the valg test that ensures failures are detected was somewhat broken, due to compiler optimizations (I think the compiler was able to detect that the condition i<10 was always going to be verified).
NOTE: this PR does touch stuff outside of eamxx, so it will have to undergo nightly testing through next.
This would mean a loss of some coverage, right? What's the alternate solution?
Technically, yes. But the only alternative is to stuff the code with lots of if/ifdef, to make sure padding in packs is always valid. This is not much different from what we do for FPE.
I think, as a double safety, we could make sure that packs and views in debug are always initialized with quiet_NaN() (or, more generally, with ekat::ScalarTraits<T>::invalid()). This would allow us to catch errors when pack size > 1 in regular debug builds (since if we ever access those garbage values to compute physical entries, we'd get nans). But I think that may require some time, and we should prob do it in a separate PR?
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I was able to fix our valgrind failures by doing a few mods. Namely:
pow(UNINITED,exponent). The altearnative is to ensure the base is always inited, but there may be quite a few ramifications.se_XYZnamelist variables in HOMME before reading them, in case they are missing in the input namelist. Usually, they are missing if not used (e.g., no point in settingse_nefor DP runs, sincese_ne_xandse_ne_ywill be used), but valgrind doesn't know that. @oksanaguba I added you as a reviewer for the handful of lines in namelist mod, just in case.i<10was always going to be verified).NOTE: this PR does touch stuff outside of eamxx, so it will have to undergo nightly testing through next.