Materialize the DFT axis default when converting opset 19 to 20 - #3023
Open
Mohammed Alkindi (MohammedAlkindi) wants to merge 1 commit into
Open
Materialize the DFT axis default when converting opset 19 to 20#3023Mohammed Alkindi (MohammedAlkindi) wants to merge 1 commit into
Mohammed Alkindi (MohammedAlkindi) wants to merge 1 commit into
Conversation
Opset 19 defines DFT axis as an attribute defaulting to 1. Opset 20 moved it to an input defaulting to -2. dft_19_20 read the attribute with a None default and returned early when it was absent, so the node was left untouched while the model opset was bumped, silently retargeting the transform from axis 1 to axis -2 for any input of rank greater than 3. The None guard is kept because _get_int_attribute also returns None for an attribute of unexpected type.
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.
Opset 19 defines DFT
axisas an attribute defaulting to 1. Opset 20 moved it to an input defaulting to -2.dft_19_20read the attribute with aNonedefault and returned early when it was absent, so the node was left untouched while the model opset was bumped. For any input of rank greater than 3 the converted model silently computes a different transform.Measured on a rank-4 input with no
axisattribute, numpy as ground truth, at the parent commit:onnx 1.22.0's own converter emits an axis Constant of 1 for this model, so the fix makes onnxscript agree with it.
The existing DFT tests all set
axisexplicitly, so the default path was never exercised. The added test omits it and fails on the unpatched source with1 != 3.version_converter,optimizerandrewriter: 573 passed, 2 skipped, 6 xfailed.The
Noneguard stays:_get_int_attributealso returnsNonefor a wrongly-typed attribute. Open PR #2941 changes only the@registerdecorator on this function, so the two do not overlap.