Fix type_conversion, unused_label, and unitialized warnings in f_west.f - #15
Open
d-diaz wants to merge 1 commit into
Open
Fix type_conversion, unused_label, and unitialized warnings in f_west.f#15d-diaz wants to merge 1 commit into
d-diaz wants to merge 1 commit into
Conversation
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
Clears 70 gfortran
-Wallwarnings inf_west.f(67-Wconversion, 2-Wunused-label, 1-Wmaybe-uninitialized). These updates produce no numerical change. Outputs are bit-identical across a sweep of test cases for the Flewelling westside model implemented on the fork of this repo.The warning
Three distinct patterns:
DATAliterals (32).SHP_W3andSHP_W4declare their regional coefficient tables asREAL*4 r25(8), r34(8)but fill them withd0double-precision literals. gfortran flags each literal because the stored single-precision value differs in binary from the double it was written as.IMPLICIT DOUBLE PRECISION (A-H,O-Z)for intermediates while the public interface (RFLW,RHFW,Z,DIBact) isREAL*4. Every store back across that boundary is an implicit narrowing conversion. The output variables were all declared as REAL*4. so no precision is being lost compared to current behavior, just making that narrowing explicit.FDBT_C1has branches forJSP3, 4 and 5 but noELSE, so any otherJSPreturns with the function result never assigned. Labels40and50inside it are branch targets nothing jumps to.The fixes
1.
DATAliterals — single-precision suffixes.Before:
After:
The stored value is unchanged: rounding the decimal to
REAL*4directly gives the same bit pattern as rounding it toREAL*8first and then narrowing. I checked all 32 literals — none is a double-rounding boundary case.2. Narrowing stores — explicit
REAL().Before:
After:
This makes the existing narrowing explicit. The arithmetic is untouched — in particular the
D0literals insideCOR_WS's expressions are left as-is, since dropping them would change the expression's evaluation precision rather than just its final conversion.3.
FDBT_C1— initialize the result; drop the dead labels.Callers only ever pass
JSP3–5 (sf_shp.f), so no reachable behavior changes; the out-of-range path simply becomes deterministic instead of returning whatever was on the stack.One warning is intentionally left:
geosubis unused inSHP_W5because western redcedar has no regional coefficients. It can be dropped from the signature and still operate, but doing so would disrupt the pattern thatsf_shp.fcallsSHP_W3,SHP_W4andSHP_W5through one uniform interface, so a comment explaining that was added instead.Test evidence
The westside Flewelling path had no automated coverage on the fork, so new tests were added first, then used to verify the code updates proposed here.
F00–F08FW2/FW3× Douglas-fir 202, western hemlock 263, western redcedar 242)vol[]outputs over 7,290 input combinations (9 GEOSUB × 3 species × 3 model paths × 10 diameters × 9 heights), before vs. afterSHP_W3/W4/W5store and 4,320 of ther25/r34lookups, and deliberately perturbing theSF_ZFD3/SF_ZFD4stores and ther25/r34tables changes 3,240 and 507 cases respectivelylint,test,warningsjobs — see the fork PR linked belowHow to reproduce
Before: 71 warnings. After: 1 (the documented
geosubdummy argument).Additional context
d-diaz/VolumeLibrary#14Two latent issues were found while working here and are not touched by this PR — happy to file them as issues if useful:
SHP_W4readsf(48)at line 263, but that routine'sDATAstatements only initializef(45)–f(47). Static storage means it currently reads as zero, so behaviour is stable, but the intended coefficient appears to be missing. I did not guess a value, since any substitution would change results.FDBT_C1,RATIOis assigned only insideIF(GCODE(ID).EQ.GEOSUB)within theDO 100/DO 200loops, so aGEOSUBmatching none of'01'–'08'leaves it unset. gfortran does not flag this one.