fix: Initialize unassigned total_soil_depth in soilLiqFlx.f90#621
Open
DarriEy wants to merge 1 commit intoCH-Earth:develop_sundialsfrom
Open
fix: Initialize unassigned total_soil_depth in soilLiqFlx.f90#621DarriEy wants to merge 1 commit intoCH-Earth:develop_sundialsfrom
DarriEy wants to merge 1 commit intoCH-Earth:develop_sundialsfrom
Conversation
The local variable `total_soil_depth` is declared (line 865) but never
assigned in `update_surfaceFlx_liquidFlux_computation_max_infiltration_rate`.
It is used in the Green-Ampt and TOPMODEL infiltration formulae, where
it appears in denominators (e.g., `wettingFrontSuction/depthWettingFront`
with `depthWettingFront = (rootZoneLiq/availCapacity) * min(rootingDepth,
total_soil_depth)`).
On platforms where the stack is not zero-initialized (e.g., Linux x86-64
with gfortran 13), `total_soil_depth` reads as 0.0, making
`depthWettingFront = 0.0` and causing 0/0 = NaN in the infiltration
rate computation. This NaN propagates through the flux vector into
`computResid`, producing the error:
FATAL ERROR: .../computResid/NaN in residuals
On macOS/ARM the bug is masked because the stack memory at that address
happens to contain a non-zero value from a previous call.
Fix: Compute `total_soil_depth = sum(in_surfaceFlx % mLayerDepth)`
before the first use, consistent with how the upstream master/develop
branches use `sum(mLayerDepth)` inline.
Contributor
|
Thanks Darri -- this is now fixed so we can close this without merging. @wknoben |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
total_soil_depthinupdate_surfaceFlx_liquidFlux_computation_max_infiltration_rate(soilLiqFlx.f90)total_soil_depth = sum(in_surfaceFlx % mLayerDepth), consistent with howsum(mLayerDepth)is used inline in the master/develop branchesProblem
The Green-Ampt and TOPMODEL infiltration formulae divide by
total_soil_depth(viadepthWettingFront). When the variable is uninitialized:0.0, causingdepthWettingFront = 0.0and0/0 = NaNin the infiltration rate. The NaN propagates through the flux vector intocomputResid:Diagnosis
Debug prints added around the Green-Ampt computation confirmed:
Building with
-finit-real=snanconfirms the variable is never assigned — the entire column 5 of the Jacobian becomes signaling NaN.Test plan
infRateMax=GreenAmpt,nrgConserv=enthalpyForm)total_soil_depthvalue matches expected soil column depth (1.0 m for test domain)🤖 Generated with Claude Code