Skip to content

Fix REAL(8) to REAL(4) conversion and tab warnings in f_alaska.f - #17

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

Fix REAL(8) to REAL(4) conversion and tab warnings in f_alaska.f#17
d-diaz wants to merge 1 commit into
FMSC-Measurements:masterfrom
d-diaz:upstream/fix-f_alaska

Conversation

@d-diaz

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

Copy link
Copy Markdown
Contributor

Summary

Clears 40 of the 41 gfortran -Wall warnings in f_alaska.f (29 -Wconversion, 11 -Wtabs) by making the already-intended narrowing conversions explicit with REAL() and replacing hard tabs with spaces. No numerical change. Warning cleanup only.

This follows the same pattern as #15 (f_west.f) and #16 (f_other.f).

The warning

All 29 conversion warnings are byte-identical apart from position:

f_alaska.f:113:16: Warning: Possible change of value in conversion from REAL(8) to REAL(4) at (1) [-Wconversion]
f_alaska.f:163:10: Warning: Possible change of value in conversion from REAL(8) to REAL(4) at (1) [-Wconversion]
f_alaska.f:372:13: Warning: Possible change of value in conversion from REAL(8) to REAL(4) at (1) [-Wconversion]

plus 11 driver-level warnings:

f951: Warning: Nonconforming tab character in column 1 of line 102 [-Wtabs]
f951: Warning: Nonconforming tab character in column 1 of line 233 [-Wtabs]
f951: Warning: Nonconforming tab character in column 1 of line 368 [-Wtabs]

Each conversion site stores a REAL*8 coefficient or intermediate into a REAL*4 scalar. gfortran flags the implicit narrowing because it can silently lose precision. Here the narrowing is deliberate — the source says so at f_alaska.f:162, immediately above nine of the sites:

c                                        (These outcomes are SINGLE precision)

The fix

One edit class: wrap the right-hand side in REAL() at the point of assignment.

Coefficient unpack (VAR_AK, 16 sites of this shape):

-      VA00 = V(1, JSPR)
+      VA00 = REAL(V(1, JSPR))

Logit block (SHP_AK) — these six lines are byte-identical to the SHP_W* routines in f_west.f, so this is the same edit already proposed in #15:

-      R1= dexp(U1)/ (1.0d0 + dexp(U1))
+      R1= REAL(dexp(U1)/ (1.0d0 + dexp(U1)))

Function result (COR_AK) — note the statement label in columns 1–5 is preserved:

-100   COR_AK = CORR
+100   COR_AK = REAL(CORR)

And each of the 11 tabs:

-<TAB>IF(JRSP .EQ. 4) JRSP = 3
+      IF(JRSP .EQ. 4) JRSP = 3

This is a provable no-op: the compiler already emitted exactly this conversion. The declarations are unchanged, so the stored value is the same REAL(4) rounding of the same REAL(8) expression — REAL() only states in source what the compiler was doing silently. The tab replacement is likewise exact: gfortran's fixed-form extension already advances a column-1 tab to column 7, so substituting 6 spaces leaves every effective column unchanged. The sweep below is confirmation.

One site deserves a reviewer's attention. SHP_AK:113-114 and VAR_AK:393 compute the same median-diameter intermediate as f_west.f:74-75 and f_other.f:301-302,634-635 — but those files declare DMEDIAN/DFORM/DRATIO as REAL*8 (f_west.f:33,171,321, f_other.f:184,521), while f_alaska.f:33,313 declares them REAL*4. That difference is the entire reason these three lines warn here and the identical statements elsewhere do not. Widening the declarations to match the sibling files is arguably the correct fix and would improve accuracy, but it changes every downstream U1U9 value, so it was not changed here — flagged below instead.

What was deliberately not changed:

  • No declaration widening (REAL*4REAL*8) — that would change results.
  • SHP_AK:114 is not rewritten from 1.0d0 to 1.0e0. That literal sits in an arithmetic expression, not a DATA initializer, so it promotes the whole expression to double precision; respelling it would evaluate in single precision and can move results. Same trap as f_other.f:876 in Fix REAL(8) to REAL(4) conversion warnings in f_other.f #16. It is wrapped in REAL() and left otherwise alone.

Test evidence

On the development fork...

Check Result
Automated tests 86/86 passed, including 18 new golden cases that specifically exercise this file
Values compared Bit-identical (IEEE-754 byte comparison, not a tolerance) on all 15 returned volume components across 3,300 cases
Tolerance None needed — exact equality
Warning count f_alaska.f 41 → 1; no new warnings anywhere

Details:

  • Sweep. Pre-edit and post-edit shared libraries were loaded side by side and called with identical inputs over 3,300 cases: all 12 equation strings that dispatch into this file (JSP 31–36 — Alaska yellow-cedar, western redcedar, and spruce/hemlock in both old-growth and second-growth form) crossed with a grid of dbh (6–45 in), total height (30–150 ft), merchantable tops, stump height, and upper-stem point including one below breast height. Every one of the 15 vol[] components matched bit for bit, as did errflag.
  • The sweep harness was itself validated. Perturbing a single F coefficient in its sixth decimal place (0.36073443D+010.36073543D+01) produced 539 mismatches, and they fell exactly in the two species-042 equations — the precise blast radius for that coefficient — confirming the comparison detects changes at the level being claimed.
  • New tests. 18 golden cases were added: 12 covering all four SHP_AK coefficient sets and all six FDBT_AK species branches across both the 2-point and 3-point entry paths, plus 6 that assert the Region 10 equation lookup returns the strings those cases use. A gcov build confirmed these cases execute all 29 edited lines — so the bit-identity result covers every line touched, not just the file in aggregate. Only one edited line is conditional (SHP_AK:168, under IF (U5 .le. 7.0d0)); it records 16 executions against 24 for its neighbours, so both branches are genuinely exercised.

How to reproduce

With nothing but a stock gfortran, confirm the warnings are gone:

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

Before this change that prints 41 warnings; after, one remains — Unused dummy argument 'setopt' in FDBT_AK. That argument is left in place deliberately: sf_shp.f:28 dispatches FDBT_AK through the same signature as the other FDBT_* bark routines, so removing it would break the call. Happy to address it separately if you would prefer it gone.

Additional context

Not required reading to review this PR:

  • This change is source-only. Test goldens, warning baselines, and build tooling live on the development fork and are not proposed here.
  • Fork development PR: d-diaz/VolumeLibrary#19
  • Fork CI run: view logs here — the warnings job shows the 41 → 1 drop; the test job's Run pytest step shows the 18 new f_alaska_* and getvoleq_r10_* cases.
  • This branch is cut from upstream/master. f_alaska.f there is byte-identical to the file the fix was developed and tested against, so the diff below is exactly the change that was verified.

Separate issues that should be considered for later resolution (not fixed here)

While working in this file I found issues whose fixes would each change numerical output or dispatch behavior. None belong in a no-op warning PR. Happy to open issues or follow-up PRs if useful:

  1. SHP_AK corrupts its own coefficient table. f_alaska.f:107-111 patches the DATA-initialized (and therefore implicitly SAVEd) F array in place when GEOSUB=='01' for spruce/hemlock, and never restores it. The first such call overwrites F(25,3), F(34,3), F(42,3) for the life of the process, so every later JSP 33/34 call silently returns second-growth results regardless of its own geosub. Measured: after a single A01F32W098 call, a subsequent A00F32W098 moves from 107.5 to 113.3 cuft (+5.4%) and 450 to 510 bdft (+13.3%), with two product fields dropping to zero. It appears currently unreachable through the shipped tables — the only A01 equations in voleqdef.f are A01BRUW202, A01DEMW000, A01DVEW094, A01DVEW375 and A01DVEW747, none of which satisfy VOLEQ(4:4)=='F' — so this is latent rather than live bug. f_ingy.f:280-300 solves the same problem correctly by copying into a scratch column that is rebuilt on every call. Worth asking: is the geosub-01 second-growth submodel meant to be reachable at all? If it is obsolete, deleting the block may be better than fixing it.
  2. DMEDIAN/DFORM/DRATIO are narrower here than in the sibling files. f_alaska.f:33,313 declares them REAL*4; f_west.f:33,171,321 and f_other.f:184,521 use REAL*8. Alaska therefore rounds the median-diameter intermediate to single precision and feeds that loss through the whole U1U9 chain. This looks like an oversight rather than a decision.
  3. sf_shp.f:50 uses .OR. where every sibling guard uses .AND.ELSEIF(JSP.GE.31 .OR. JSP.LE.36) is unconditionally true. Compare sf_shp.f:26, sf_corr.f:25, sf_dfz.f:24, sf_3pt.f:116. It is last in the ELSEIF chain so currently reachable JSP values still route correctly, but any value not caught earlier (1, 2, 6–10, 37+) falls into SHP_AK and indexes F(10, JSP-30) out of bounds.
  4. Dead branch. f_alaska.f:130 tests IF(JRSP .eq. 15) for a Lodgepole Pine (INGY) model, but JRSP is remapped to 1–4 at lines 101–104 and can never reach 15.
  5. Stale header comment. f_alaska.f:3-6 lists the file's routines as SHP_AK, COR_AK, VAR_AK — omitting FDBT_AK.

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