[Fix][Relax][Frontend][Torch] Validate num_classes in the one_hot converters - #20320
Open
siyiweigeHEW wants to merge 1 commit into
Open
[Fix][Relax][Frontend][Torch] Validate num_classes in the one_hot converters#20320siyiweigeHEW wants to merge 1 commit into
num_classes in the one_hot converters#20320siyiweigeHEW wants to merge 1 commit into
Conversation
… converters
`_one_hot` forwards `num_classes` straight to `relax.op.one_hot` without any
validation, so a non-positive value reaches the C++ builder and trips its
internal `depth > 0` check:
InternalError: Check failed: (depth > 0) is false:
one_hot: depth must be positive, but got 0
The message never mentions `num_classes` and gives no hint on how to fix the
model. `num_classes` is a plain constant, so it is accepted as-is by both
`torch.export.export` and `fx.symbolic_trace`, and the failure surfaces only
when the graph is lowered through TVM.
Reject a non-positive `num_classes` in the frontend instead, on both the
`from_fx` and the `from_exported_program` paths. torch's `num_classes=-1`
(infer the depth from the input) cannot be supported here because the
resulting depth is data dependent, so the error message says so explicitly.
Add regression tests for both frontends.
Reported in apache#20319.
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.
Fixes: #20319
Summary
The Relax PyTorch frontend's
_one_hotconverter reads thenum_classesargument of an
F.one_hot/aten.one_hotcall and forwards it verbatim torelax.op.one_hot. It performs no validation at all, so a non-positivenum_classesreaches the C++ op builder(
src/relax/op/tensor/manipulate.cc), which asserts:The message never mentions
num_classesand gives no hint about how to fix themodel. This is reachable because
num_classesis an ordinary constant:both
torch.export.exportandfx.symbolic_traceaccept it as-is and recordit in the graph, so the failure only appears once the graph is lowered through
TVM.
This PR makes the frontend reject a non-positive
num_classeswith a clearValueErrorthat names the argument, on both converter copies.Root cause
_one_hotexists twice — once for the legacyfrom_fxpath(
fx_translator.py) and once for the modernfrom_exported_programpath(
exported_program_translator.py). Both validate only thatnum_classeswasfound, then pass it straight through:
num_classesis a static attribute ofrelax.op.one_hot(it determines theoutput depth), so the frontend is the only place that can report the problem
usefully. torch itself only rejects an invalid
num_classeswhen the model isexecuted:
num_classestorch.exportfx.symbolic_trace5(3, 5)(3, 5)0RuntimeErrorInternalError: depth must be positive, but got 0-1(explicit)max+1)InternalError: depth must be positive, but got -1-2RuntimeErrorInternalError: depth must be positive, but got -2num_classes=-1is torch's documented "infer the depth from the input" value.That cannot be honoured here — the depth would be data dependent and
relax.op.one_hot's depth is static — so it is rejected as well, with an errormessage that says why.
Fix
Both copies of
_one_hot(
python/tvm/relax/frontend/torch/fx_translator.pyandpython/tvm/relax/frontend/torch/exported_program_translator.py) gain the samecheck right after the existing "argument missing" guard:
The
isinstance(num_classes, int)guard keeps the change conservative: anynon-literal
num_classesthat may legitimately be dynamic is left untouched, soonly the reported constant case changes behaviour.
This mirrors the existing frontend-side validation style already used elsewhere
in the same files (e.g.
_flatten_impl'sstart_dim/end_dimchecks inbase_fx_graph_translator.py), and applies to both entry points, sincefrom_fxusesfx_translatorandfrom_exported_programusesexported_program_translator.Validation
In-tree regression tests (added)
test_one_hot_invalid_num_classesintests/python/relax/test_frontend_from_fx.py—num_classes ∈ {0, -1, -2}are rejected with
ValueErrorbefore lowering; the valid case is alreadycovered by the existing
test_one_hot.test_one_hot_invalid_num_classesintests/python/relax/test_frontend_from_exported_program.py—num_classes=0is rejected on thefrom_exported_programpath withrun_ep_decomposition=False.Both tests fail without the fix (
tvm.error.InternalError: Check failed: (depth > 0) is false: one_hot: depth must be positive, but got 0) and pass withit.
Note on the modern path:
from_exported_programdecomposesaten.one_hottoarange/equal/astypeby default (run_ep_decomposition=True), so theconverter is normally bypassed and the test therefore passes
run_ep_decomposition=False. Passing that flag is a supported, already-testedconfiguration (see
test_einsum), and with it_one_hotis live code that hitsexactly the same C++ check.
Behaviour after the fix
num_classes ∈ {3, 5, 10}viafrom_fx: output shape and values matchnative PyTorch exactly (
max|diff| = 0);num_classes ∈ {0, -1, -2}viafrom_fx:ValueError: one_hot num_classes must be a positive integer, but got 0. Inferring the depth from the input (torch's num_classes=-1) is not supported because the resulting depth is data dependent.num_classes=0viafrom_exported_program(run_ep_decomposition=False): sameValueError.Full-suite run
tests/python/relax/test_frontend_from_fx.pyandtests/python/relax/test_frontend_from_exported_program.pywere run in fullwith the change (390 tests: 372 passed, 16 failed, 2 skipped) and again with the
change reverted (372 passed, 16 failed, 2 skipped, 2 deselected). The two
failure sets are identical, so the change introduces no regressions. The 16
failures are pre-existing and unrelated to
one_hot(test_extended_unary_ops,test_interpolate,test_select_slice,test_masked_select,test_to_copy,test_index_put,test_eye,test_cross_entropy, thetest_dynamic_shape*family,
test_sym_size_int,test_stochastic_depth); they come from the localtest tree being newer than the source/lib build used for the run, and were
verified to fail identically before and after the change.
Files changed
python/tvm/relax/frontend/torch/fx_translator.py— validatenum_classesin
_one_hot.python/tvm/relax/frontend/torch/exported_program_translator.py— validatenum_classesin_one_hot.tests/python/relax/test_frontend_from_fx.py— addtest_one_hot_invalid_num_classes.tests/python/relax/test_frontend_from_exported_program.py— addtest_one_hot_invalid_num_classes.