Repository navigation
[Fix][Relax][PyTorch] Handle one-sided clamp on integer tensors - #20885
Open
JakkulaVeerababu wants to merge 1 commit into
Open
JakkulaVeerababu wants to merge 1 commit into
JakkulaVeerababu wants to merge 1 commit into
Conversation
The clamp, clamp_min and clamp_max converters fill a missing bound with +/-inf and pass it to relax.op.clip, which cannot represent an infinite bound for an integer dtype, so the result is garbage. Use the integer type limits for a missing bound instead. clamp_max also read its bound from args[2], so a positional max was ignored. Read it from args[1]. Fixes apache#20881 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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 #20881
TVM was handling PyTorch clamp operations incorrectly for integer tensors. When one limit was missing, it used infinity, which
relax.op.clipcannot represent for an integer dtype, so the output was garbage. Also,clamp_maxwas reading the maximum value from the wrong argument (args[2]instead ofargs[1]), so a positional value was ignored.This PR adds a common helper,
_emit_clip, used by_clamp,_clamp_minand_clamp_max. For integer tensors it now uses the actual integer limits of the dtype instead of infinity. It also fixes theclamp_maxargument, and addstest_clamp_one_sided_integer, which checksclamp(min=...),clamp(max=...),clamp_minandclamp_maxagainst PyTorch for int32, int64 and uint8.Testing:
tests/python/relax/test_frontend_from_exported_program.py: 257 passed, 2 skipped.test_normfails with and without this change (amax.defaultis unsupported with the newer PyTorch I used locally).tests/python/relax/test_frontend_from_fx.py -k "clamp or unary": 27 passed.AI disclosure: I used Claude Code to help find the cause, write the fix and the test, and run the tests. I reviewed the change and can explain it.