Skip to content

Fix type_conversion, unused_label, and unitialized warnings in f_west.f - #15

Open
d-diaz wants to merge 1 commit into
FMSC-Measurements:masterfrom
d-diaz:upstream/fix-f_west
Open

Fix type_conversion, unused_label, and unitialized warnings in f_west.f#15
d-diaz wants to merge 1 commit into
FMSC-Measurements:masterfrom
d-diaz:upstream/fix-f_west

Conversation

@d-diaz

@d-diaz d-diaz commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Clears 70 gfortran -Wall warnings in f_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

f_west.f:57:38: Warning: Change of value in conversion from 'REAL(8)' to 'REAL(4)' [-Wconversion]
f_west.f:119:10: Warning: Possible change of value in conversion from REAL(8) to REAL(4) [-Wconversion]
f_west.f:930:4:  Warning: Label 40 at (1) defined but not used [-Wunused-label]
f_west.f:968:72: Warning: '__result_fdbt_c1' may be used uninitialized [-Wmaybe-uninitialized]

Three distinct patterns:

  1. DATA literals (32). SHP_W3 and SHP_W4 declare their regional coefficient tables as REAL*4 r25(8), r34(8) but fill them with d0 double-precision literals. gfortran flags each literal because the stored single-precision value differs in binary from the double it was written as.
  2. Narrowing stores (35). These routines use IMPLICIT DOUBLE PRECISION (A-H,O-Z) for intermediates while the public interface (RFLW, RHFW, Z, DIBact) is REAL*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.
  3. FDBT_C1 has branches for JSP 3, 4 and 5 but no ELSE, so any other JSP returns with the function result never assigned. Labels 40 and 50 inside it are branch targets nothing jumps to.

The fixes

1. DATA literals — single-precision suffixes.

Before:

      real*4 m1, m2, m3, R25(8), R34(8)
      data (r25(i),i=1,8)/   -2.0262D0, -1.7945d0, -2.0366d0,
     >         -2.0811d0, -1.9868d0, -2.0151d0, -1.9475d0, -2.0151d0 /

After:

      real*4 m1, m2, m3, R25(8), R34(8)
      data (r25(i),i=1,8)/   -2.0262E0, -1.7945e0, -2.0366e0,
     >         -2.0811e0, -1.9868e0, -2.0151e0, -1.9475e0, -2.0151e0 /

The stored value is unchanged: rounding the decimal to REAL*4 directly gives the same bit pattern as rounding it to REAL*8 first and then narrowing. I checked all 32 literals — none is a double-rounding boundary case.

2. Narrowing stores — explicit REAL().

Before:

      R1= dexp(U1)/ (1.0d0 + dexp(U1))
      A3=U6
      Z  = GAMMA + DELTA*LOG(Y/(1.0d0-Y))

After:

      R1= REAL(dexp(U1)/ (1.0d0 + dexp(U1)))
      A3=REAL(U6)
      Z  = REAL(GAMMA + DELTA*LOG(Y/(1.0d0-Y)))

This makes the existing narrowing explicit. The arithmetic is untouched — in particular the D0 literals inside COR_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.

c              JSP outside 3-5 will not set FDBT_C1, so we initialize it here.
      FDBT_C1 = 0.0

Callers only ever pass JSP 3–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: geosub is unused in SHP_W5 because western redcedar has no regional coefficients. It can be dropped from the signature and still operate, but doing so would disrupt the pattern that sf_shp.f calls SHP_W3, SHP_W4 and SHP_W5 through 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.

Check Result
Automated tests 47/47 passed, including 33 new Flewelling westside cases (equations F00F08 FW2/FW3 × Douglas-fir 202, western hemlock 263, western redcedar 242)
Values compared All 15 vol[] outputs over 7,290 input combinations (9 GEOSUB × 3 species × 3 model paths × 10 diameters × 9 heights), before vs. after
Tolerance Exact — bit-for-bit identical on every value; no tolerance needed
Coverage confirmed Verified the comparison actually reaches the edited lines: gcov shows 4,860 executions of each SHP_W3/W4/W5 store and 4,320 of the r25/r34 lookups, and deliberately perturbing the SF_ZFD3/SF_ZFD4 stores and the r25/r34 tables changes 3,240 and 507 cases respectively
Fork CI run lint, test, warnings jobs — see the fork PR linked below

How to reproduce

gfortran -Wall -c f_west.f -o /dev/null

Before: 71 warnings. After: 1 (the documented geosub dummy argument).

Additional context

  • Fork development PR: d-diaz/VolumeLibrary#14
  • This PR is source-only; baselines and tooling live on the fork.

Two latent issues were found while working here and are not touched by this PR — happy to file them as issues if useful:

  • SHP_W4 reads f(48) at line 263, but that routine's DATA statements only initialize f(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.
  • In FDBT_C1, RATIO is assigned only inside IF(GCODE(ID).EQ.GEOSUB) within the DO 100/DO 200 loops, so a GEOSUB matching none of '01''08' leaves it unset. gfortran does not flag this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant