Tell apart torch overloads that TorchScript reports under one node kind - #2809
Open
LeSingh1 wants to merge 1 commit into
Open
Tell apart torch overloads that TorchScript reports under one node kind#2809LeSingh1 wants to merge 1 commit into
LeSingh1 wants to merge 1 commit into
Conversation
TorchScript drops the overload suffix, so several aten ops reach the converter as one kind with two different argument layouts. Three of them assume the wrong layout. In each case the two overloads have different arity, which is what the fix keys on. sum / mean: aten::sum(self, dtype) has 2 inputs, aten::sum.dim_IntList( self, dim, keepdim, dtype) has 4. The converter always read position 1 as dim, so torch.sum(x, dtype=torch.float32) passed the dtype enum as an axis and failed with "IndexError: pop index out of range". randint: aten::randint(high, size, ...) has 6 inputs, aten::randint.low( low, high, size, ...) has 7. Under TorchScript the converter always took the .low layout, so torch.randint(10, (2, 3)) read the size list as high and the dtype enum as the shape, failing with "TypeError: 'int' object is not iterable". round: aten::round.decimals(self, decimals) has 2 inputs and was rejected outright by expected=1. Accept it and scale by 10**decimals, since dropping the argument would silently change the result.
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.
TorchScript drops the overload suffix, so several
atenops reach the converter as a single node kind with two different argument layouts. Three converters assume the wrong one. In each case the overloads differ in arity, which is what the fix keys on.torch.sum(x, dtype=torch.float32)aten::sum, 2 inputsIndexError: pop index out of rangetorch.mean(x, dtype=torch.float32)aten::mean, 2 inputstorch.randint(10, (2, 3))aten::randint, 6 inputsTypeError: 'int' object is not iterabletorch.round(x, decimals=2)aten::round, 2 inputsValueError: node 5 (round) got 2 input(s), expected [1]sum / mean —
aten::sum(self, *, dtype)has 2 inputs;aten::sum.dim_IntList(self, dim, keepdim, dtype)has 4. The converter always read position 1 asdim, so the dtype enum (6for fp32) was passed as an axis. The torch.export branch already discriminates by node kind; only the TorchScript branch was missing it.randint —
aten::randint(high, size, ...)has 6 inputs;aten::randint.low(low, high, size, ...)has 7. Under TorchScript the converter unconditionally took the.lowlayout, reading the size list ashighand the dtype enum as the shape.round —
aten::round.decimals(self, decimals)was rejected outright byexpected=1. Accepted now, anddecimalsis honored by scaling by10**decimals; dropping it would silently change the result.decimalsis also read from keyword inputs, for the export path.Verified arities by tracing, e.g.
torch.sum(x)andtorch.sum(x, dtype=...)are both 2 inputs whiletorch.sum(x, dim=0),torch.sum(x, 0, True)andtorch.sum(x, dim=0, dtype=...)are all 4, so arity separates them cleanly.logsumexp(3),all/any(1 or 3) are unaffected.Testing
TestSum::test_sum_mean_dtype,TestRandint::test_randint_no_low,TestElementWiseUnary::test_round_decimals(decimals 0, 1, 2, -1). All 13 TorchScript cases fail onmainand pass here. ExistingTestSum,TestRandintandTestElementWiseUnarytests still pass.This machine has a broken scikit-learn install that makes every
TorchFrontend.TORCHEXPORTcase error out, including pre-existing ones such asTestElementWiseUnary::test_acosh, so I could only exercise the TorchScript frontend locally.