Skip to content

Fix REAL(8) to REAL(4) conversion warnings in f_other.f - #16

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

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

Conversation

@d-diaz

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

Copy link
Copy Markdown
Contributor

Summary

Clears all 54 gfortran -Wall warnings in f_other.f (53 -Wconversion, 1 -Wtabs) by making the already-intended narrowing conversions explicit with REAL() and replacing one hard tab with spaces. No numerical change — verified bit-identical over a 12,960-case sweep. Warning cleanup only.

The warning

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

f_other.f:613:13: Warning: Possible change of value in conversion from REAL(8) to REAL(4) [-Wconversion]
f_other.f:356:10: Warning: Possible change of value in conversion from REAL(8) to REAL(4) [-Wconversion]
f_other.f:876:19: Warning: Possible change of value in conversion from REAL(8) to REAL(4) [-Wconversion]

plus one driver-level warning:

f951: Warning: Nonconforming tab character in column 1 of line 70 [-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_other.f:355, immediately above nine of the sites:

c                                        (These outcomes are SINGLE precision)

The distribution across the file's seven compile units:

Unit Warnings
BRK_OT 9
SHP_OT 9
COR_OT 3
VAR_OT 18
SHP_BH 9
COR_BH 3
VAR_BH 2

The fix

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

Coefficient unpack (VAR_OT, 34 sites of this shape):

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

Multi-line expression — the closing paren goes on the continuation line (COR_OT, COR_BH):

-       CORR = exp( q1*(t4-t3) + q2*(t4*t4-t3*t3)/2.0D0
-     >                  +q3*(t4*t4*t4 - t3*t3*t3)/3.0D0)
+       CORR = REAL(exp( q1*(t4-t3) + q2*(t4*t4-t3*t3)/2.0D0
+     >                  +q3*(t4*t4*t4 - t3*t3*t3)/3.0D0))

And the tab at line 70:

-<TAB>D2 = BK(9,JSPR)
+      D2 = BK(9,JSPR)

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. That is the whole semantic argument; the sweep below is confirmation, not the proof.

What was deliberately not changed:

  • No declaration widening (REAL*4REAL*8) — that would change results.
  • No reflowing or inserted lines. Every edit is in place on an existing line: the file is 905 lines before and after, and the diff is 60 insertions / 60 deletions.
  • VAR_BH:876,882 are not rewritten from D+01 to E+01. Those literals sit in an arithmetic expression, not a DATA initializer, so the double literal promotes the whole expression to double precision; respelling them as E+01 would evaluate in single precision and can move results. They are wrapped in REAL() and left otherwise alone.

A few lines needed minor internal spacing tightened (e.g. Q1 * (T4 - T3)Q1*(T4 - T3)) purely to keep the statement within fixed-form column 72 after adding REAL(.

Test evidence

Check Result
Automated tests 74/74 passed, including 20 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 12,960 cases
Tolerance None needed — exact equality
Warning count f_other.f 54 → 0; repo total 2,038 → 1,984; no new warnings anywhere

Details:

  • Sweep. The pre-edit and post-edit shared libraries were loaded side by side and called with identical inputs over 12,960 cases: all 18 equation strings that dispatch into this file (JSP 22–29, covering Regions 2, 3, 4/Dixie and the Black Hills NF) crossed with a grid of dbh (6–45 in), total height (25–140 ft), merchantable tops, stump height, and upper-stem point. Every one of the 15 vol[] components matched bit for bit, as did errflag.
  • The sweep harness was itself validated. Perturbing a single BK coefficient in its sixth decimal place (0.6996780.699679) produced 1,160 mismatches, confirming the comparison detects changes at the level being claimed.
  • New tests. 20 golden cases were added covering every JSP 22–29 route, both the 2-point and 3-point entry paths, and each region-specific bark sub-model. A gcov build confirmed these 20 cases execute all 53 edited lines — so the bit-identity result covers every line touched, not just the file in aggregate.

How to reproduce

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

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

Before this change that prints 54 warnings; after, none.

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#17
  • Fork CI run: view logs here — the warnings job shows the 54 → 0 drop; the test job's Run pytest step shows the 20 new f_other_* cases.
  • This branch is cut from upstream/master (release 20260731). f_other.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 found issues whose fixes would each change numerical output. None belong in a no-op warning PR. Happy to open issues or follow-up PRs if useful:

  1. BK loses precision at initialization. BK(9,8) is REAL*8 (f_other.f:24) but its DATA literals carry no exponent letter, so 28 values with 8+ significant digits are truncated to single precision before being widened — 12.88990159 stores as 12.8899002…. gfortran cannot see this; the widening itself is lossless.
  2. DBHIB can be used uninitialized. f_other.f:84-127 is an IF/ELSE IF chain with no ELSE, so a JSPR outside 1–8 reaches DBTBH = DBHOB - DBHIB at line 128 with DBHIB unset.
  3. Unguarded division. Line 140 divides by DBTBH, which line 128 can leave at 0; B2-DR**B3 is likewise unguarded. An in-source DW 08/22 comment at line 137 proposes a filter, and line 139 implements it for DR only.
  4. Dispatch guards exceed the coefficient arrays. sf_shp.f:48, sf_corr.f:22 and sf_dfz.f:18 guard JSP.LE.30JSPR = JSP-22, reaching 8 against 7-column F/V; brk_up.f:10 and calcdia.f:535 guard JSP.LE.30JSPR = JSP-21, reaching 9 against BK(9,8). fwinit.f:157-235 only ever emits 22–29, so this is latent, not live.
  5. Stale comments. f_other.f:578 labels the V(*,5) block REGION 2 WHITE PINE while lines 48, 253 and 431 all call index 5 White fir. fwinit.f:340-364's JSP→model table omits 28, 29 and 33–36.

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