fix(dynamo): keep x & False and x | True out of TensorRT - #4579
Open
shoumikhin wants to merge 2 commits into
Open
fix(dynamo): keep x & False and x | True out of TensorRT#4579shoumikhin wants to merge 2 commits into
x & False and x | True out of TensorRT#4579shoumikhin wants to merge 2 commits into
Conversation
`bitwise_and.Scalar`, `bitwise_or.Scalar` and their `Scalar_Tensor` forms return wrong values on TensorRT 11.2 when the scalar is the one that fixes the result of the op: False for AND, True for OR. This is what turned four bitwise converter tests red on main. `x & False` is False whatever x is, and `x | True` is True whatever x is, so TensorRT folds the layer down to a constant. It gets that wrong when the constant operand is smaller than the output and has to broadcast. A Python scalar always reaches the network as a rank-0 constant, so it always has to broadcast, so those two combinations always hit it. Checked directly against the TensorRT API, outside torch-tensorrt, on 11.2.1.2: a bool constant of shape (1, 1, 1) against a (5, 3, 2) bool tensor aborts the build with an internal error for AND with False and for OR with True, and is correct for every other combination, including both XOR cases. Giving the constant the full output shape, so that nothing has to broadcast, is correct in all cases. Inside torch-tensorrt the same graph builds but the engine returns garbage. The capability validator already rejects the Tensor overload when `other` is rank-0, for the same underlying reason. Extend it to the two scalar combinations that are actually broken, and only those, so the partitioner keeps them in PyTorch while the rest still go to TensorRT. Tests: the validator unit tests now cover all eight scalar combinations of AND/OR/XOR and both scalar overloads. The scalar converter tests keep the values that TensorRT gets right, which is what proves the fallback is not wider than the problem. The two broken values move to new tests that compile through the full pipeline, since the converter harness bypasses the partitioner and never consults the validator; they assert that no engine is built and that the result matches eager.
lanluo-nvidia
self-requested a review
August 26, 2026 05:22
The repository lint job runs `black --check .` across the whole tree, so any file that does not match the formatter fails CI for every open pull request, not only the one that touched it. `tests/py/dynamo/conversion/test_cumsum_aten.py` is currently not black-conformant on main, which turns the Python Linting check red here. Reformat that one file with black. This is a formatting-only change: two statements that fit on a single line are un-wrapped. No test logic changes. Verified by running `black --check .` on the full tree: all files pass.
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.
What is broken
Four converter tests are red on
main:TestBitwiseAndConverter::test_bitwise_and_scalar_3dTestBitwiseAndConverter::test_bitwise_and_scalar_tensor_3dTestBitwiseOrConverter::test_bitwise_or_scalar_2dTestBitwiseOrConverter::test_bitwise_or_scalar_tensor_2dThey fail on the value, not with an error:
Mismatched elements: 5 / 30. The AND cases are the ones parameterized withFalseand the OR cases are the ones parameterized withTrue. The other scalar value passes in both cases. This started with the upgrade from TensorRT 11.1 to 11.2.Why
x & FalseisFalsewhateverxis, andx | TrueisTruewhateverxis, so TensorRT folds the layer down to a constant. It gets that fold wrong when the constant operand is smaller than the output and has to broadcast. A Python scalar always reaches the network as a rank 0 constant, so it always has to broadcast, so those two combinations always hit it.Checked directly against the TensorRT API on 11.2.1.2, outside torch-tensorrt: a bool constant of shape
(1, 1, 1)against a(5, 3, 2)bool tensor aborts the build with an internal error for AND withFalseand for OR withTrue, and is correct for every other combination, including both XOR cases. Give the same constant the full output shape, so nothing has to broadcast, and all cases are correct. Inside torch-tensorrt the build succeeds and the engine returns garbage.So this is a TensorRT issue, and the two combinations that hit it are exactly the two that are red.
Fix
The capability validator already rejects the
Tensoroverload whenotheris rank 0, for the same underlying reason. Extend it to the two scalar combinations that are broken, and only those, so the partitioner keeps them in PyTorch. Everything else still goes to TensorRT.Tests
The validator unit tests now cover all eight scalar combinations of AND, OR and XOR across both scalar overloads. The scalar converter tests keep the values TensorRT gets right, which is what shows the fallback is not wider than the problem. The two broken values move to new tests that compile through the full pipeline, because the converter harness bypasses the partitioner and never consults the validator; they assert that no engine is built and that the result matches eager.
All six new tests were checked to fail when the validator change is removed, and the five bitwise test files pass together with it (51 tests).