Skip to content

Materialize the DFT axis default when converting opset 19 to 20 - #3023

Open
Mohammed Alkindi (MohammedAlkindi) wants to merge 1 commit into
microsoft:mainfrom
MohammedAlkindi:fix/dft-axis-default
Open

Materialize the DFT axis default when converting opset 19 to 20#3023
Mohammed Alkindi (MohammedAlkindi) wants to merge 1 commit into
microsoft:mainfrom
MohammedAlkindi:fix/dft-axis-default

Conversation

@MohammedAlkindi

Copy link
Copy Markdown
Contributor

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. For any input of rank greater than 3 the converted model silently computes a different transform.

Measured on a rank-4 input with no axis attribute, numpy as ground truth, at the parent commit:

converted DFT inputs: ['input_x']
opset-19 result == numpy fft(axis=1): True
opset-20 result == numpy fft(axis=2): True
max abs diff: 4.825716018676758

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 axis explicitly, so the default path was never exercised. The added test omits it and fails on the unpatched source with 1 != 3.

version_converter, optimizer and rewriter: 573 passed, 2 skipped, 6 xfailed.

The None guard stays: _get_int_attribute also returns None for a wrongly-typed attribute. Open PR #2941 changes only the @register decorator on this function, so the two do not overlap.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

1 participant