Allocate the SaveAt buffer from the saved value, not its shape - #767
Open
nstarman wants to merge 1 commit into
Open
Allocate the SaveAt buffer from the saved value, not its shape#767nstarman wants to merge 1 commit into
SaveAt buffer from the saved value, not its shape#767nstarman wants to merge 1 commit into
Conversation
`jnp.full` conjures an array with no operand, so an array-ish abstraction wrapping `y0` -- a `quax.Value` carrying units, an uncertainty, a sparsity pattern -- has nothing to dispatch on and is erased as soon as a saved value round-trips through the buffer via the `lax.cond` in `_save`. Route the same `inf` fill through a select whose predicate is a compile-time constant, so `y` is an operand. XLA folds the select away and DCEs the dead branch, including the `subsaveat.fn` call feeding it: the cost analysis is unchanged (4 fewer flops, identical bytes accessed and temp memory) and saved values, including the `inf` fill of unwritten slots, are bit-identical. The `stop_gradient` is load-bearing -- without it the buffer acquires a zero-valued but structurally present tangent path back to `y0`, tripping `BacksolveAdjoint`'s `nondifferentiable` guard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
nstarman
force-pushed
the
quax-saveat-buffer
branch
from
August 19, 2026 03:34
51c3151 to
29ecb3b
Compare
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.
Allocates the
SaveAtoutput buffer via an operation on the value being saved, rather than conjuring it from that value's shape and dtype.The problem
_allocate_outputbuilds the buffer withjnp.full:jnp.fulltakes no operand. So wheny0is an array-ish abstraction — aquax.Valuecarrying units, an uncertainty, a sparsity pattern — there is nothing here for Quax to dispatch on, and the buffer comes out as a plain array._savethen puts a slice of that plain buffer opposite the carried value in alax.cond:The two branches disagree on structure,
cond_pneeds one, and the wrapper is dropped.sol.yscomes back as a bare array; a type that declines to materialise (units, say — a dimensionless metre has no meaning) raises instead.Why this one can't be fixed from outside
In #438 the conclusion was that Quax shouldn't require changing Diffrax, and for essentially all of
diffeqsolvethat holds — Quax intercepts primitives, and Diffrax is written in primitives. This site is the exception, and structurally so: a rule can only fire where there is an operand to dispatch on, and an array conjured from a shape has none. No set of Quax rules can reach it.(The workaround in #438 — a
defaultrule permissive enough to re-box everything — only appears to work because it also boxes the solver's integer loop counters, at which point almost every primitive has a boxed operand and the buffer gets re-boxed on first write. That is also whysol.tscame back wrapped there.)The change
Keep the
inffill exactly, but route it through aselectwhose predicate is a compile-time constant, soyis an operand:XLA folds the select away and DCEs the dead branch — including the
subsaveat.fncall feeding it.The
stop_gradientis load-bearing. Without it the buffer acquires a tangent path back toy0— zero-valued, but structurally present — andBacksolveAdjointtrips its ownnondifferentiableguard:RuntimeError: Unexpected tangent. That showed up as 4 failures intest_adjoint.py. Withstop_gradientall adjoints produce gradients identical tomain.Cost
XLA cost analysis, 64-dim state,
Tsit5+PIDController(rtol=1e-6, atol=1e-8),max_steps=4096:SaveAtt1=Truesteps=Truesteps=True, fn=sin(y)**3+cos(y)Four fewer flops, identical memory traffic. The non-trivial
fnrow is there to show the extrasubsaveat.fncall costs nothing once the dead branch is eliminated.Behaviour
Unchanged. Saved values are bit-identical and unwritten slots still read back as
inf— includingtest_saveat_solution.py's explicitjnp.allclose(ys1, jnp.array([0.25, 0.09375, jnp.inf])).One caveat worth naming:
subsaveat.fnis now invoked once during allocation. It was already invoked inside the solve, so this introduces no new kind of behaviour, but afnwith a side effect (ajax.debug.print, say) would fire one additional time, since DCE removes the pure computation but not the effect.Two simpler forms that I tried and discarded, in case they come to mind on review:
jnp.broadcast_to(y_init, ...)with no select at all. Changes the fill frominftoy0, which contradicts the assertions above, and separately broketest_adjoint.py::test_against.stop_gradient, which breaksBacksolveAdjointas described.Test
test/test_quax.pyassertssol.yskeeps its wrapper acrosst1/t0,t1/ts/steps, thatsol.tsdoes not pick one up, and that theinffill survives. Verified against this branch: 4 failures before the change, all green after.It is gated on
quax>=0.5.0and so currently skips —quaxify(diffeqsolve)also needs Quax'scustom_vjpsupport (Diffrax's buffered loops route throughjax.custom_vjp), which lands in that release. Happy to drop the test and the test-group dependency if you would rather not carry a skipped test until then.Not fixed here
With the default
throw=True, theSolutionis passed throughequinox.internal.error_if, which allocates its error branch fromjax.eval_shape+pure_callback— the same shape-not-operand pattern, one library over. That erases the type again, so the test usesthrow=False. Filed as patrick-kidger/equinox#1257, with a candidate patch.