Fp16 constant weights - #3015
Open
mloubout wants to merge 12 commits into
Open
Conversation
`__getitem_hook__` matched on `_defines` overlap alone. A derived Dimension carries its parent in `_defines`, so for a Bundle indexed by `(p_rec, rp_recx)` -- `rp_recx` being a `CustomDimension` whose parent is `p_rec` -- the lookup for `rp_recx` matched the `p_rec` entry first and returned the number of sparse points where the number of interpolation weights was meant. That size becomes the innermost stride in `_generate_fsz`, so the receiver kernels of a vectorized Operator read `w[p*npoint + rp]` instead of `w[p*2 + rp]` and run off the end of the array. Observed as an out-of-bounds `__global__` read under compute-sanitizer and a run-to-run varying, sometimes NaN, elastic TTI gradient on CUDA. Try an exact hit before falling back to the overlap, in both `__getitem_hook__` and `dindex`.
`Add` reports its first argument's `indices_ref`, so a sum whose terms sit at different staggered locations names a position only one of them has, and `x0` gets resolved against it for all of them. The shear strain `v_x.dy + v_y.dx` of a staggered velocity is the canonical case: both terms land on the cell corner, so a shift onto it should be a no-op, and instead each picked up a spurious one. Differentiation is linear at every order, so split such a sum in `Derivative._eval_fd`. Relative error on `D(a+b)` against `D(a) + D(b)` was 0.63 at order 0, 1.20 at order 1 and 0.95 at order 2, with `expand=False` at order 2 returning exactly zero. `generic_derivative` also short-circuited a zeroth order derivative only when `x0` was empty, building a stencil around an expression already sitting at `x0`. `index_at` answers where an expression sits, and both call sites use it.
A LocalObject carries expressions in its constructor arguments and in its initializer, and both end up in the generated code, but FindApplications only visited Expressions, Iterations and Calls. Any macro they apply was therefore left undefined -- ROUND_UP, say, for an auto-padded stride reaching a plan descriptor.
The Weights of a non-expanded derivative were always built at the default precision, so a `float16` stencil got `float` coefficients. Every wavefield*weight product then bound to the mixed-precision operators and was promoted, defeating the point of the half-precision wavefield.
`_gen_value` printed the initializer with the printer's default dtype rather than the Array's, which stamped a `float` suffix onto the entries of a `double` Array and silently rounded them to single precision. Route it through a new `initvalue` printer hook, which also gives the targets a place to specialize an initializer whose type cannot be built from a plain literal.
`_prec` floors an untyped real literal at `float32` so that an integer default doesn't degrade the arithmetic around it. That floor also caught `float16`, which is never a fallback but an explicit request, so every literal in a half-precision Operator printed one type too wide. Only apply the floor when the default is not already a real type.
The stability check sums the whole field and asks whether the result is finite. The accumulator took the field's own dtype, so in half precision it overflowed within a few thousand points and reported an instability that wasn't there -- making `errctl=max`, the very option one reaches for to diagnose a suspected instability, unusable exactly where it is needed. Give it at least single precision.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3015 +/- ##
==========================================
- Coverage 83.68% 83.00% -0.68%
==========================================
Files 257 257
Lines 54711 54835 +124
Branches 4686 4695 +9
==========================================
- Hits 45785 45518 -267
- Misses 8115 8469 +354
- Partials 811 848 +37
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:
|
A real literal in an otherwise integer expression is emitted at the Operator's precision, floored at `float32` so that an integer default does not degrade it. An Operator working in half wants that floor most of the time -- half is a storage format, and the accuracy of the literals is worth more than the width of the multiply -- but not always. Give the printer a flag for it, off by default, and have `_printer` pick up a Target's second printer where one is offered.
The same coefficients at two precisions are two different arrays, but neither `__eq__` nor `_hashable_content` looked at the dtype, so the first one built answered for both. An Operator asking for its weights in one precision would be handed whichever an earlier Operator had cached. Compare and hash on it. The name goes in rather than the type itself, which does not order and so cannot be sorted alongside the rest.
Whether an Operator working in half also computes in half decides what is calculated, not how quickly: the literals and the FD coefficients are rounded to three decimal digits. That is a mathematical choice, so it belongs with `interp-mode` in `sym_opt` rather than among the codegen options, and is validated and defaulted alongside it.
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
1 participant
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.
On top of #3012
TODO:
Add a knob for fp16 weights