Add version converter adapters for opset 15, 16, 17, 18#7677
Add version converter adapters for opset 15, 16, 17, 18#7677
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds version converter adapters to enable downgrading ONNX models from higher opsets (18, 17, 16) to opset 15, which is necessary for deployment on Unity Sentis. The PR addresses two main issues: bfloat16 type support added in opset 16, and the reduction attribute added/extended in opsets 16 and 18 for Scatter operations.
Changes:
- Adds adapters for downgrading 16->15 (TypeRestriction for bfloat16 and RemoveAttribute for reduction)
- Adds adapters for downgrading 18->17 (rejection of "max"/"min" reduction values)
- Refactors
bfloat16_not_allowedvector definition to be reused across multiple opset transitions - Extends
RemoveAttributetransformer to support string value validation
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| onnx/version_converter/convert.h | Registers new adapters for 16->15 and 18->17 conversions, moves bfloat16_not_allowed definition for reuse |
| onnx/version_converter/adapters/transformers.h | Adds RemoveAttribute overload for string value validation |
| onnx/version_converter/adapters/scatter_18_17.h | New adapter that rejects "max"/"min" reduction values when downgrading to opset 17 |
| onnx/version_converter/adapters/scatter_16_15.h | New adapter that enforces type restrictions and removes reduction attribute when downgrading to opset 15 |
| onnx/test/version_converter_test.py | Adds comprehensive tests for Where, ScatterElements, and ScatterND conversions between opsets 15-18 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7677 +/- ##
==========================================
+ Coverage 55.29% 55.35% +0.06%
==========================================
Files 515 515
Lines 32466 32513 +47
Branches 2900 2901 +1
==========================================
+ Hits 17951 17998 +47
Misses 13725 13725
Partials 790 790 ☔ View full report in Codecov by Sentry. |
16 -> 15: - Where: TypeRestriction (bfloat16) - ScatterElements: ScatterElements_16_15 (bfloat16 + reduction "none" only) - ScatterND: ScatterND_16_15 (bfloat16 + reduction "none" only) - Scan: TypeRestriction (bfloat16) - GreaterOrEqual: TypeRestriction (bfloat16) - LeakyRelu: TypeRestriction (bfloat16) - PRelu: TypeRestriction (bfloat16) 18 -> 17: - ScatterElements: ScatterElements_18_17 (reject reduction "max"/"min") - ScatterND: ScatterND_18_17 (reject reduction "max"/"min") Supporting changes: RemoveAttribute(Symbol, string) in transformers.h; scatter_16_15.h, scatter_18_17.h; tests in version_converter_test.py. Signed-off-by: alexkvitkevich <alexander.kvitkevich@tripledotstudios.com>
Signed-off-by: alexkvitkevich <alexander.kvitkevich@tripledotstudios.com>
### Motivation and Context Models rarely specify a domain in practice. The spec required it (MUST), creating a disconnect between specification and reality. Changed to SHOULD to align with actual usage while preserving the reverse domain name convention guidance. Changed one word in `docs/IR.md` line 92: `MUST` → `SHOULD`. <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>[Spec] Relax wording for domain name for models</issue_title> > <issue_description>In spec > > > Models MUST specify a domain and use reverse domain names based on the responsible organization’s identity, the same convention that is used for [...] > > In reality, models are rarely specified a domain and that is fine. Relax the wording to "SHOULD".</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes onnx#7667 <!-- START COPILOT CODING AGENT TIPS --> --- ✨ Let Copilot coding agent [set things up for you](https://github.com/onnx/onnx/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: justinchuby <11205048+justinchuby@users.noreply.github.com> Signed-off-by: alexkvitkevich <alexander.kvitkevich@tripledotstudios.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Alexander Kvitkevich <100673758+alexkvitkevich@users.noreply.github.com> Signed-off-by: alexkvitkevich <alexander.kvitkevich@tripledotstudios.com>
Added a note to include the release manager in the ONNX organization for wiki write permissions. ### Motivation and Context Fixes # --------- Signed-off-by: Andreas Fehlner <fehlner@arcor.de> Co-authored-by: Ti-Tai Wang <titaiwang@microsoft.com> Signed-off-by: alexkvitkevich <alexander.kvitkevich@tripledotstudios.com>
Signed-off-by: alexkvitkevich <alexander.kvitkevich@tripledotstudios.com>
a90333b to
72f0428
Compare
Signed-off-by: alexkvitkevich <alexander.kvitkevich@tripledotstudios.com>
Signed-off-by: alexkvitkevich <alexander.kvitkevich@tripledotstudios.com>
Motivation and Context
Recent PyTorch only supports exporting ONNX at higher opsets (>18), not 15. To deploy on Unity Sentis, which only supports opset between 7 and 15, we use the ONNX version converter to downgrade the exported model. Without the new adapters, that downgrade fails or is incorrect when the graph contains
WhereorScatterElements/ScatterND, because the converter had no 16 -> 15 and 18 -> 17 adapters forbfloat16and the'reduction'attribute. This PR adds those adapters so conversion from PyTorch’s export opset down to 15 works for these ops.New adapters:
16 -> 15:
18 -> 17:
reduction = "max"/"min")reduction = "max"/"min")