opt=1: decline the int8 variant above its activation-encoding ceiling - #156
opt=1: decline the int8 variant above its activation-encoding ceiling#156axiom-of-choice wants to merge 1 commit into
Conversation
opt=1 picks on predicted cost and never measures, so nothing validated the lossy int8 variant: a matmul whose true sums sit inside fp16 range came back with inf (179/496 non-finite on M2 Pro, 0/496 at opt=0). Fixes sbryngelson#153. Takes option 1 with the fail-closed rule on an unknown bound, per the design call in the issue, and follows the three asks that came with it. Shares the existing constant rather than adding a second 4094: _targets renames FP16_SLICE_SAT to Q4_X16_SAT, naming the encoding instead of the slice op, and keeps the old name as an alias. The int8-weight matmul routes activations through that same Q.4 x16 encoding, which is why the ceiling does not move with weight scale, K, N or seed. Keeps the ceiling per variant, not global: _variant_act_ceiling is keyed by config flag and returns None for variants with no encoding limit, so int4-LUT and sparse can declare their own. Bounds activations across the whole graph. _node_max_abs only ever bounded one node; _act_max_abs walks to the leaves and returns None if any is a runtime input, which _exceeds_act_ceiling treats as unsafe, mirroring the slice-saturation rule at _targets.py:215. On M2 Pro the repro goes from 179/496 non-finite to 0/496, matching opt=0. int8 still survives at opt=1 for a const-fed graph bounded under the ceiling, so this is a gate rather than a ban, and opt=2 is untouched since it validates by measurement. 10 new build-level tests run in CI without an ANE and pin 4094 including the exact 4094/4096 boundary. Corpus GATE GREEN 90/90.
|
This is an excellent implementation -- exactly the design we converged on: per-variant I'm going to hold it, though -- and it's purely a sequencing call, nothing about the code. As So let's land #155 first, then this, together: provably-bounded and range-declared graphs keep the fast path, and only genuinely-unbounded activations lose int8 at opt=1. The fix is right; I just don't want to ship the regression window in between. #155 is the more interesting piece anyway, and I'll review it as soon as you pick it up. One nit for when this comes back: the test docstring references |
|
Understood on the sequencing, and the reasoning is right: trading a narrow correctness bug for a broad throughput regression is the wrong order when the recovery path does not exist yet. Picked up #155: #161. It carries this gate plus the bound machinery, so it supersedes this PR rather than stacking, and there is no window where int8 is off for everything. The part that turned out to matter more than the declaration itself: the saturating activations bound an undeclared input, so Also fixed your nit there: Happy to close this one in favour of #161, or leave it open if you would rather diff the two. Your call. |
|
Closing in favor of #161, which carries this same opt=1 ceiling gate plus the #155 bound machinery -- so there's no window where int8 is off for everything, and any graph downstream of a saturating op (softmax/sigmoid/tanh/l2_norm/relu6) keeps the fast path with no annotation. The design here was exactly right; #161 is the same idea with the recovery path attached. Thanks -- see the review over there (one rebase needed so it doesn't revert #157). |
I built this branch's _optimize.py from the sbryngelson#156 branch, which predates sbryngelson#157, so copying it reverted the fail-closed relerr check. sbryngelson#157's source-form test caught it in CI, which is what that test exists for. Verified the rest of the copy carried nothing else stale: the diff against main is now only the ceiling gate and the rename.
…vably safe (#161) * compile: declare input activation ranges so opt=1 keeps int8 when provably safe Implements #155, and supersedes #156 by carrying its gate plus the bound machinery that makes the gate an optimization instead of a near-blanket decline. af.input(shape, max_abs=) records a promise about |value| at runtime. Undeclared inputs stay unbounded, so the fail-closed default from #153 is unchanged. _propagate_max_abs walks the graph conservatively. Anything not modelled returns None rather than a guess, since over-estimating only declines a lossy variant more often while under-estimating would wrongly keep one. Covers non-expanding elementwise ops, scalar mul/add, add/sub/mul/min/max over both operands, the matmul and linear weight row-sum bound, bmm, clip by its limits, and the saturating activations that are bounded whatever feeds them. That last group matters more than the declaration itself: sigmoid, tanh, softmax, l2_norm and relu6 bound an undeclared input, so anything downstream of a softmax or a sigmoid recovers int8 at opt=1 with no annotation at all. _act_max_abs now bounds the activation feeding each weight-consuming node rather than the graph output, which is the quantity a weight-quantizing variant actually has to encode. Verified on M2 Pro: the repro goes from 179/496 non-finite to 0/496, and a graph declaring max_abs=8.0 with real activations peaking at 7.1 keeps int8 and stays finite at relerr 4.6e-3, against 2.5e-4 for the lossless path. 21 new build-level tests, the 10 from #156 still pass, corpus GATE GREEN 90/90. Also commits bench/opt1_int8_saturation_repro.py, which the review on #156 asked for, and fixes the test docstring that referenced it while it was absent. * Restore the merged fail-closed gate in _optimize I built this branch's _optimize.py from the #156 branch, which predates #157, so copying it reverted the fail-closed relerr check. #157's source-form test caught it in CI, which is what that test exists for. Verified the rest of the copy carried nothing else stale: the diff against main is now only the ceiling gate and the rename.
2 participants
Fixes #153. Implements option 1 with the fail-closed rule on an unknown bound, per your design call, plus the three asks that came with it.
The bug
opt=1selects on predicted cost and never measures (_compile._compile_opt), so nothing validated the lossy int8 variant. A matmul whose true sums sit inside fp16 range came back withinf:The three asks
Share the constant, don't add a second 4094.
_targets.FP16_SLICE_SATbecomesQ4_X16_SAT, naming the encoding rather than the slice op, with the old name kept as an alias so nothing outside breaks. Your invariance result is what justifies this: the ceiling not moving with weight scale, K, N or seed means the int8-weight matmul is routing activations through the same Q.4 x16 encoding, so one constant is correct rather than a coincidence.Per-variant, not global.
_variant_act_ceiling(cfg)is keyed by config flag and returnsNonefor variants with no encoding limit, so int4-LUT and sparse can declare their own ceilings without inheriting int8's. A test pins that{"int8": False}and const-fold reportNone.Pin 4094 in a regression test. Done, including the exactness argument:
np.float16(4095) == np.float16(4096), so no fp16 activation lies between 4094 and 4096 and|act| <= 4094is a gate with no boundary gap. Parametrized either side of it.How the bound is computed
_node_max_absonly ever bounded a single node, so it could not answer "what is the largest activation in this graph". Added_act_max_abs, which walks to the leaves and returnsNoneif any leaf is a runtimeinput._exceeds_act_ceilingthen treatsNoneas unsafe, mirroring the precedent you pointed at (_targets.py:215):_variants(out, drop_unsafe=True)filters on that, and only theopt=1path passes the flag.opt=2is untouched, since it validates by measurement and was already safe.Verification
Apple M2 Pro (Mac14,12 Mac mini), macOS 26.5.2.
The repro is fixed, and
opt=1now matchesopt=0exactly:It is a gate, not a ban. int8 still survives at
opt=1where it is safe:inputNonetests/test_opt1_act_ceiling.pytest_routes,test_tune_guards,test_compile_targetstests/run_corpus.pyruff/pyright aneforge/ pre-commitBehaviour gap confirmed on
mainthrough the public API rather than by a stashed-file run, which only produces a collection error:_variantshas nodrop_unsafeparameter there and offers int8 unconditionally.On the cost, honestly
You flagged that on any graph with runtime inputs the bound is
None, so this declines int8 atopt=1nearly always, landing close to option 3 in practice. That matches what I see, and I have not measured the throughput given up on realopt=1workloads either, so I would not claim the cost is negligible. Two things make me still prefer this shape: it keeps int8 available for const-fed subgraphs today, and it is the right structure once input ranges can be declared, which is the change that makes this meaningfully different from option 3. Happy to switch to option 2 (single probe dispatch, reject on non-finite) if you would rather trade a dispatch for the throughput.Declaring an input range looks worth its own issue, as you said. Say the word and I will file it with this PR as context.
Not in this PR
The fail-closed
not (relerr <= tol)change on theopt=2path, which you asked for as a separate small PR since it is independently merge-worthy rather than a fix for this. Sending that next.