Fix REAL(8) to REAL(4) conversion warnings in f_other.f - #16
Open
d-diaz wants to merge 1 commit into
Open
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 all 54 gfortran
-Wallwarnings inf_other.f(53-Wconversion, 1-Wtabs) by making the already-intended narrowing conversions explicit withREAL()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:
plus one driver-level warning:
Each conversion site stores a
REAL*8coefficient or intermediate into aREAL*4scalar. gfortran flags the implicit narrowing because it can silently lose precision. Here the narrowing is deliberate — the source says so atf_other.f:355, immediately above nine of the sites:c (These outcomes are SINGLE precision)The distribution across the file's seven compile units:
BRK_OTSHP_OTCOR_OTVAR_OTSHP_BHCOR_BHVAR_BHThe 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):Multi-line expression — the closing paren goes on the continuation line (
COR_OT,COR_BH):And the tab at line 70:
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 sameREAL(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:
REAL*4→REAL*8) — that would change results.VAR_BH:876,882are not rewritten fromD+01toE+01. Those literals sit in an arithmetic expression, not aDATAinitializer, so the double literal promotes the whole expression to double precision; respelling them asE+01would evaluate in single precision and can move results. They are wrapped inREAL()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 addingREAL(.Test evidence
f_other.f54 → 0; repo total 2,038 → 1,984; no new warnings anywhereDetails:
JSP22–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 15vol[]components matched bit for bit, as diderrflag.BKcoefficient in its sixth decimal place (0.699678→0.699679) produced 1,160 mismatches, confirming the comparison detects changes at the level being claimed.JSP22–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:
Before this change that prints 54 warnings; after, none.
Additional context
Not required reading to review this PR:
warningsjob shows the 54 → 0 drop; thetestjob'sRun pyteststep shows the 20 newf_other_*cases.upstream/master(release 20260731).f_other.fthere 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:
BKloses precision at initialization.BK(9,8)isREAL*8(f_other.f:24) but itsDATAliterals carry no exponent letter, so 28 values with 8+ significant digits are truncated to single precision before being widened —12.88990159stores as12.8899002…. gfortran cannot see this; the widening itself is lossless.DBHIBcan be used uninitialized.f_other.f:84-127is anIF/ELSE IFchain with noELSE, so aJSPRoutside 1–8 reachesDBTBH = DBHOB - DBHIBat line 128 withDBHIBunset.DBTBH, which line 128 can leave at 0;B2-DR**B3is likewise unguarded. An in-sourceDW 08/22comment at line 137 proposes a filter, and line 139 implements it forDRonly.sf_shp.f:48,sf_corr.f:22andsf_dfz.f:18guardJSP.LE.30→JSPR = JSP-22, reaching 8 against 7-columnF/V;brk_up.f:10andcalcdia.f:535guardJSP.LE.30→JSPR = JSP-21, reaching 9 againstBK(9,8).fwinit.f:157-235only ever emits 22–29, so this is latent, not live.f_other.f:578labels theV(*,5)blockREGION 2 WHITE PINEwhile 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.