Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Split index bugs uncovered by QNN SDK 2.19 #19381

Merged
merged 2 commits into from
Feb 2, 2024

Conversation

adrianlizarraga
Copy link
Contributor

@adrianlizarraga adrianlizarraga commented Feb 2, 2024

Description

  • When converting ONNX split sizes to QNN split indices, do not include the split at index 0. QNN 2.19 assumes index 0 is implicit and throws a validation error if provided.
  • Fix bug when using an ONNX Split operator with a num_outputs attribute that does not evenly divide into shape[axis]. The ONNX spec states that the last chunk should be smaller, but QNN EP made the last chunk larger.
  • Fix bug when using an ONNX Split operator with a split input. QNN EP was incorrectly passing the split sizes as split indices without conversion.

Motivation and Context

QNN SDK 2.19 updated validation criteria for Split operators. QNN EP was previously passing a split index that should have been implicit.

Also, discovered a bugs when using num_outputs attribute and split input.

@adrianlizarraga adrianlizarraga marked this pull request as ready for review February 2, 2024 01:03
Copy link
Contributor

@HectorSVC HectorSVC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@adrianlizarraga adrianlizarraga changed the title Fix Split index bug uncovered by QNN SDK 2.19 Fix Split index bugs uncovered by QNN SDK 2.19 Feb 2, 2024
@jywu-msft jywu-msft merged commit a2eb967 into main Feb 2, 2024
92 of 94 checks passed
@jywu-msft jywu-msft deleted the adrianl/qnn-split-indices-fix branch February 2, 2024 08:22
@sophies927 sophies927 added the triage:approved Approved for cherrypicks for release label Feb 7, 2024
YUNQIUGUO pushed a commit that referenced this pull request Feb 8, 2024
### Description
- When converting ONNX split sizes to QNN split indices, do not include
the split at index 0. QNN 2.19 assumes index 0 is implicit and throws a
validation error if provided.
- Fix bug when using an ONNX Split operator with a `num_outputs`
attribute that does not evenly divide into `shape[axis]`. The ONNX spec
states that the last chunk should be smaller, but QNN EP made the last
chunk larger.
- Fix bug when using an ONNX Split operator with a `split` input. QNN EP
was incorrectly passing the split sizes as split indices without
conversion.

### Motivation and Context
QNN SDK 2.19 updated validation criteria for Split operators. QNN EP was
previously passing a split index that should have been implicit.

Also, discovered a bugs when using `num_outputs` attribute and `split`
input.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:1.17.1 triage:approved Approved for cherrypicks for release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants