fix(defn): accept constant tensor arguments in Nx.block - #1807
Conversation
| # A block's arguments may mix traced expressions with plain constant tensors | ||
| # (for example `Nx.LinAlg.solve(x, b)` where `b` is closed over). Constants | ||
| # carry no context of their own, so take the context from the first traced | ||
| # argument as the fallback for those constants. | ||
| defp block_context(args) do | ||
| Enum.find_value(args, :root, fn | ||
| %T{data: %Expr{context: context}} -> context | ||
| _ -> nil | ||
| end) | ||
| end |
There was a problem hiding this comment.
I think this might have a slight bug, though I'm not sure if it's actually reproducible: If we have a block inside a while using a closure from outside the while, the order in which they appear might result in a hidden invalid context:
defn foo_lhs(a) do
x = a + 1
while {x, i = 0}, i < 10 do
x_next = Nx.LinAlg.solve(a, x)
{x, i + 1}
end
end
defn foo_rhs(a) do
x = a + 1
while {x, i = 0}, i < 10 do
x_next = Nx.LinAlg.solve(x, a)
{x_next, i + 1}
end
endNeither of these should work, and I have a feeling that the second one will end up passing through.
There was a problem hiding this comment.
Both fail correctly in the branch, which is good. I am pushing a simplified implementation along with these as tests.
expr_block/3 built one parameter per argument via parameter/2, which matches only expressions and so had no clause for a constant tensor. Nx.LinAlg.solve/2 is the only multi-argument block op today, so it was the visible victim: jitting a solve against a closed-over right-hand side raised FunctionClauseError. Convert every argument to an expression so the block node stores expressions the tree traversal can walk, and derive a fallback context from the first traced argument for constants only. Arguments that already carried their own context keep exactly that context, matching pre-fix behavior; only constants (which have none) fall back to the derived one. Closes elixir-nx#1805
2c96e83 to
00ef161
Compare
| @rhs Nx.tensor([4.0, 3.0, 2.0]) | ||
| @mat Nx.tensor([[4.0, 1.0, 0.5], [1.0, 3.0, 0.2], [0.5, 0.2, 2.0]]) | ||
|
|
||
| @tag compiler: Evaluator |
There was a problem hiding this comment.
I don't think it makes sense to have these tensors as module attributes
| ) | ||
| end | ||
|
|
||
| @constant_rhs Nx.tensor([4.0, 3.0, 2.0]) |
There was a problem hiding this comment.
We shouldn't have this as a module attribute. If you really need to reuse the value, either pass it down as a defn option or have it in the describe setup as a shared value.
Closes #1805.
Nx.block/4raised whenever one of its arguments was a plain constant tensor rather than a traced expression.Nx.LinAlg.solve/2is the only multi-argument block op today, so it is where this surfaced: solving against a closed-over right-hand side failed.This is not gradient-specific — it reproduces under plain
Nx.Defn.jit/1:Present in released 0.13.0 and 0.13.1 as well as
main.Cause
expr_block/3built one parameter per argument withparameter/2, which matches only%T{data: %Expr{context: context}}— it derives the tracing context from the argument itself, so a constant tensor had no matching clause.Fix
Two parts, both needed:
block_context/1), and give it only to arguments that carry no context of their own. Arguments that were already expressions keep their own context exactly as before, so the change is confined to the constants that previously had no clause at all. If nothing is traced, the fallback is:root.in_args.Part 2 is not cosmetic. With only part 1,
parameter/2stops raising but the block node still holds a raw tensor, and the failure moves toNx.Defn.Tree.scope_ids_each/3, which likewise has no clause for a non-Exprtensor.The per-argument contexts are captured before the
to_expr/1conversion, since converting erases the distinction between a traced argument and a constant.The
:rootfallback inblock_context/1is defensive rather than reachable:Nx.block/4dispatches throughNx.Shared.list_impl!(args), so an all-constant argument list resolves toBinaryBackendand never reachesexpr_block/3. Happy to drop it if you would rather not carry unreachable branches.Tests
Two, both verified failing on
mainbefore the fix:test/nx/defn_test.exs— a newdescribe "block", jitting a solve against a constant right-hand side. No test previously calledNx.block/4directly, though the linalg grad suite drives the block path throughout.test/nx/defn/grad_test.exs— in the existingdescribe "solve", asserting the constant-rhs gradient equals the traced-rhs gradient computed by the existingsolve_grad_wrt_a/2. Same mathematics, different tracing path, so any divergence is the defect.Verification
nxsuite: 2725 passed, 0 failures (2723 before, plus these two).exlasuite: 1416 passed, 0 failures, 49 excluded.mix compile --force: no new warnings.Independent of #1806 / its PR — different files, both branched from
main.