Treat discrete JAXOp inputs as disconnected in the gradient - #2418
rishabhsai wants to merge 2 commits into
Conversation
| with pytest.raises(DisconnectedInputError): | ||
| grad(out, [idx]) | ||
|
|
||
| # Blockwise consults the core Op's connection_pattern, so the same must hold |
There was a problem hiding this comment.
this is not the same, you now used a continuous input? I'd also like to see a test for wrap_jax with mixed input types. The blockwise itself was a symptom downstream, I don't think we need to test it
There was a problem hiding this comment.
Dropped the Blockwise block, agreed it was a downstream symptom. On the continuous input: the discrete assertions were the two lines above it (int32 idx, DisconnectedInputError), and the vectorized block did differentiate only the float batched_x, so it was carrying the connection_pattern coverage by accident rather than asserting anything discrete. Replaced it with test_mixed_input_types, which interleaves float x, int32 idx, float y and bool mask, checks the numeric gradients of x and y, asserts both discrete inputs raise DisconnectedInputError, and asserts connection_pattern on the op directly so that half of the fix stays covered without Blockwise.
7f52d10 to
7c85996
Compare
Description
JAXOp.pullbacksent every input throughjax.vjpand declared the VJP op's output types as[self.input_types[i] for i in range(num_inputs)], so an integer input came back with an integer gradient andpytensor/gradient.pyrejected it with "grad illegally returned an integer-valued variable". ReturningDisconnectedTypeinstead was not possible either, becauseJAXOphad noconnection_pattern, soBlockwiseassumed every input was connected and refused the disconnected term. That makesgradfail for anywrap_jaxfunction taking an index array, with or withoutpt.vectorize.Discrete inputs are now held constant in the VJP,
jax.vjponly sees the float inputs, and their gradient is reported asDisconnectedType.JAXOp.connection_patternmarks discrete inputs as disconnected from every output, whichBlockwise.connection_patternpicks up for the vectorized case. This is the same arrangementAdvancedSubtensoruses for its index inputs.Behaviour change:
gradwith respect to a discretewrap_jaxinput now raisesDisconnectedInputErrorrather than silently producing an integer zero.Related Issue
JAXOp.gradfails when inputs include integer-typed tensors #2072Checklist
Type of change