Repository navigation
Keep infinities in clamp with one bound - #3089
Open
Muhammad Usman Mateen (usmanmateen) wants to merge 1 commit into
Open
Muhammad Usman Mateen (usmanmateen) wants to merge 1 commit into
Muhammad Usman Mateen (usmanmateen) wants to merge 1 commit into
Conversation
clamp, clamp_min and clamp_max with only one bound left the other Clip input empty. ONNX Runtime uses the lowest or largest finite value of the type for an omitted bound, as the Clip spec describes, so x.clamp(min=0.5) turned inf into 3.4e38 and x.clamp(max=0.5) turned -inf into -3.4e38. Pass -inf or inf for the missing bound of floating-point inputs instead.
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.
aten_clamp(with onlyminor onlymax),aten_clamp_min,aten_clamp_maxand the 0-d tensor branches ofaten_clamp_min_tensor/aten_clamp_max_tensorleave the otherClipbound empty. TheClipspec says the bounds default tonumeric_limits::lowest()andnumeric_limits::max(), and ONNX Runtime does exactly that. So for floating-point inputs an infinity is clipped to the largest finite value:After that,
torch.isinfno longer sees the value, and something like1 / xgives2.9e-39instead of0.This adds a small
_cliphelper that passes-inf/inffor the missing bound when the input is floating point, and uses it in those five places. Integer inputs keep the omitted bound, since there the type's limits are the right default. NaN still propagates, as before.Tests
test_clamp_with_one_bound_keeps_infinitiesintests/function_libs/torch_lib/e2e_ops_tests.pyexportsclampwith onlyminand onlymax,clamp_minandclamp_maxwith a scalar, andclamp_minandclamp_maxwith a 0-d tensor bound. The input containsinf,-infandnan, and the test checks the outputs against eager. It fails onmain(infvs3.4028e+38in the first output) and passes with this change.python -m pytest tests/function_libs/torch_lib/e2e_ops_tests.py -k "clamp or clip or hardtanh or relu6": 2 passedpython -m pytest tests/function_libs/torch_lib/ops_test.py -k "clamp or hardtanh or relu6 or clip": 21 passed, 14 skipped (124 subtests passed)ruff checkandruff format --check(0.15.8) on the changed filesTested with torch 2.14.1+cpu and onnxruntime 1.31.0.