Repository navigation
Rewriter: don't fuse Min/Max into Clip when a bound has more dims than the input - #3142
Open
Shreya Boyane (Sonica-B) wants to merge 1 commit into
Open
Shreya Boyane (Sonica-B) wants to merge 1 commit into
Shreya Boyane (Sonica-B) wants to merge 1 commit into
Conversation
…n the input FuseMaxMinToClip and FuseMinMaxToClip accepted any single-element bound, including float[1] and float[1, 1]. Min and Max broadcast X up to the rank of such a bound, but the fused Clip keeps the shape of X, so the output lost dimensions. torch.clamp(x.abs().amax(), lo, hi) with buffers of shape [1] returns shape (1,) in PyTorch and in the unoptimized export, and shape () after torch.onnx.export(..., optimize=True). Skip the fusion when a bound has more dimensions than X, or when the rank of X is unknown and the bound is not a scalar. Bounds whose rank is at most the rank of X still fuse.
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.
torch.onnx.exportwith its defaults (dynamo=True,optimize=True) runs the default rewrite rules, andFuseMaxMinToClip/FuseMinMaxToClipchange the output shape oftorch.clampwhen its bounds are tensors with more dimensions than the input. A per-tensor scale such astorch.clamp(x.abs().amax(), lo, hi), withloandhiregistered as shape-[1]buffers, returns shape(1,)in PyTorch and in the unoptimized export, but a 0-d tensor after optimization, while the graph output still declares[1].onnxscript.optimizer.optimize()does the same to anyMin(Max(X, lb), ub)orMax(Min(X, ub), lb)chain with such bounds.torchlib lowers
aten::clamp.TensortoMaxfollowed byMin. The two rules accept any single-element bound (np.size(v) == 1) and emitClip(X, lb, ub)with scalar bounds.MinandMaxbroadcastXagainst afloat[1]orfloat[1, 1]bound, butClipkeeps the shape ofX. When a bound has no more dimensions thanX, broadcasting cannot change the shape, so only that case is wrong.The fix is in
_FuseMinMaxBase.check: the Clip fusion is skipped when a bound has more dimensions thanX, or when the rank ofXis unknown and the bound is not 0-d. Bounds whose rank is at most the rank ofXstill fuse, so the existing tests (which usefloat[1]bounds on 3-D and 4-D inputs) are unchanged.flowchart LR subgraph main["main"] A["Max(X: float[], lo: float[1])"] --> B["Min(., hi: float[1])<br/>output float[1]"] B -. "FuseMaxMinToClip" .-> C["Clip(X, lo, hi)<br/>output float[]"] end subgraph pr["this PR"] D["Max(X: float[], lo: float[1])"] --> E["Min(., hi: float[1])<br/>output float[1]"] E -. "check fails: lo has more dimensions than X" .-> F["graph unchanged"] endRepro (torch 2.15.0+cpu, onnxruntime 1.23.0):
Before (main @ 682b59d):
After (this branch):
On main, onnxruntime also warns when it loads the optimized model:
Error merging shape info for output. 'clamp' source:{} target:{1}. Falling back to lenient merge.The values are the same in both cases (3.0). Only the rank changes, which breaks consumers that depend on it, such asConcat,Reshapeor a caller comparing against the PyTorch output.Tests:
TestClipFusionBoundRankin_min_max_to_clip_test.pychecks both rules with a 1-D input andfloat[1, 1]bounds and with a 0-d input andfloat[1]bounds (the condition must fail), and checks that a 1-D input withfloat[1]bounds still fuses and stays numerically equal. The four failure cases fail against the rule on main, because the condition passes there, and pass on this branch.Run on Windows 11, Python 3.11, onnx 1.18.0, onnxruntime 1.23.0:
pytest onnxscript/rewriter/rules/common -q -n 4: 355 passed, 1 skipped (the existing skip for [Broken CI] test_llama_p0_rule_set_slice_split fails #1642)pytest onnxscript/optimizer -q -n 4: 87 passedlintrunneron the two changed files: no lint issuesNote:
optimizer.optimize()infers shapes before the rewrite rules run, so a graph whose intermediate values carry novalue_infostill fuses there. Only a barerewriter.rewrite()on a graph without shape information now keepsMin/Maxwhen a bound is not 0-d.