Repository navigation
dsl: Print double-precision FD weights to 20 digits - #3074
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3074 +/- ##
=======================================
Coverage 84.15% 84.15%
=======================================
Files 258 258
Lines 57267 57290 +23
Branches 4893 4893
=======================================
+ Hits 48191 48214 +23
+ Misses 8249 8248 -1
- Partials 827 828 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
182ba14 to
7e0639d
Compare
|
The two C++ CI-core failures on 43c5113 were not caused by the 20-digit literals. The literals and their hex
So main was passing the C++ jobs with 2% headroom, and any perturbation re-rolls the statistic: across 17 The last commit adds Two things this does not cover: nvc++ and nvcc also contract under safe-math (the same test fails on OpenACC |
7e0639d to
2110d1c
Compare
|
needs rebase then gtg |
| # Enforce fixed precision FD coefficients to avoid variations in results | ||
| scale = dim.spacing**(-deriv_order) if scale else 1 | ||
| weights = [sympify(scale * w).evalf(_PRECISION) for w in weights] | ||
| weights = [sympify(scale * w).evalf(_PRECISION.get(expr.dtype, 9)) for w in weights] |
There was a problem hiding this comment.
I'd rather
try:
precision = _PRECISION[expr.dtype]
except KeyError:
raise .....
weights = [sympify(scale * w).evalf(precision) for w in weights]
and regardless, no hardcoded magic numbers (eg 9 here)
There was a problem hiding this comment.
I'm sorry @FabioLuporini but no that's not better and that create a mess. THere is a bunch of cases where we have different dtypes than the standard one (with fp16 to start with) and the 9 is not magic it just defaults to fp32 precision,
There was a problem hiding this comment.
Integers do reach this site (13 tests, plus PRO's test_weights_are_real on int32/int64/float16), so a raise is a trap, but the inline 9 was duplicating the fp32 entry. Proposal: precision = _PRECISION.get(expr.dtype, _PRECISION[np.float32]), i.e. the mapper's single-precision digits for anything not listed, no raise, and a comment saying so. The current head has the strict version; I'll swap it for this if you're both happy.
There was a problem hiding this comment.
precision = _PRECISION.get(expr.dtype, _PRECISION[np.float32])
yes
A float in x0, e.g. x + 0.5*h_x, made the Taylor weights carry float noise, which only the 9-digit rounding hid.
FD weights were rounded to 9 digits whatever the dtype, which floors fp64 accuracy at ~1e-10 relative. Single precision keeps 9 digits.
GCC turns contraction off by default only in ISO C mode (-std=c99), so the C++ backend was fusing multiply-adds under safe-math while the C backend was not. With safe-math on, -ffp-contract=off makes the two match. Fast-math builds are unaffected; bitwise results change for existing safe-math C++ users.
2110d1c to
bbac36f
Compare
Take the fallback digit count from the mapper's `float32` entry instead of an inline literal. An integer or untyped expression has no floating-point precision of its own, so it and any dtype without an entry keep the nine digits their weights had before; half precision gets its own entry.
bbac36f to
bbb5de2
Compare
FD weights are rounded with
evalf(9)regardless of dtype, so fp64 operators get the same 9-digit literals as fp32 ones (e.g.2.53968254e-2for 8/315,1.196289060for 1.1962890625). In double precision this sets an accuracy floor:u.dx2ofx**2is off by ~5e-6 (should be round-off, ~5e-13);u.laplaceat space order 8 stalls at 1e-7 and gets worse as h shrinks (1.2e-4 at n=1024, vs 7.7e-11 with exact weights);Two changes:
numeric_weightsturns float literals inx0(e.g.x + 0.5*h_x) into their exact Rational value before computing the Taylor weights, like theRational(s)already used inmake_shift_x0. This removes the float noise that the 9-digit rounding was hiding, with no tolerance involved.weights=, registry rules such as drp).fp32 generated code is byte-identical to main for the seismic examples (acoustic fwd/adj/grad/born, TTI, elastic, viscoelastic, viscoacoustic; space orders 4-16). The only exception I found is
div(v, shift=(.5, .5, .5)), where main emits a spurious- 3.05311332e-16F*v_x[...]term that is now gone. Op counts are otherwise unchanged in fp64 too, and no existing expected strings had to change.This follows the review on #2977: no float-based tolerance, no extra helper or flag, and custom/drp weights are covered.
Tests:
test_shifted_div/test_shifted_gradalso run in float64 (they fail without change 1), andtest_dx2_exact_doublefails on main and passes here.Fixes #2976
🤖 Generated with Claude Code
-ffp-contract=offto GCC'ssafe-mathflags: the C++ backend was contracting multiply-adds undersafe-math while the C backend was not, which is what failed
test_gradient_equivalencein the two C++ jobs(see the comment below for the numbers).