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

Parser/printer support external data format #5688

Merged

Conversation

yocox
Copy link
Contributor

@yocox yocox commented Oct 19, 2023

Description

Improve the implementation of ONNX textual printer / parser to handle external data format

Motivation and Context

Recently the modern models get larger and larger. Models with external data become common.
The ONNX textual format is good for human writing and reading. But is dose not implement the
external data format, yet. This PR implement it.

@yocox yocox requested a review from a team as a code owner October 19, 2023 17:03
@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!.

onnx/defs/printer.cc Outdated Show resolved Hide resolved
@justinchuby
Copy link
Contributor

Could you fix DCO by following instructions at https://github.com/onnx/onnx/pull/5688/checks?check_run_id=17887346129 ? Thanks!

cc @gramalingam

@gramalingam
Copy link
Contributor

Thanks for the PR & contribution! This is great to have.

onnx/defs/parser.cc Outdated Show resolved Hide resolved
onnx/defs/parser.cc Outdated Show resolved Hide resolved
onnx/defs/printer.cc Outdated Show resolved Hide resolved
Refactoring for the following external data format support.

Signed-off-by: yoco <yoco@skymizer.com>
- Parse initializer starts with "=[" for external data
- Print initializer starts with "=[" for external data
- Add unit test

Signed-off-by: yoco <yoco@skymizer.com>
The string should use double quote, not single quote.

Signed-off-by: yoco <yoco@skymizer.com>
Signed-off-by: yoco <yoco@skymizer.com>
Signed-off-by: yoco <yoco@skymizer.com>
We cannot reference the onnx:: namespace directly
because it is compile time defined (users can choose to
call it differently).

Signed-off-by: yoco <yoco@skymizer.com>
Signed-off-by: yoco <yoco@skymizer.com>
- While print, check if the external data format is specified before the
  internall data. Theoratically they are mutually exclusize, however,
  during some conversion internal state, the external and internal might
  be specified at the same time. In that case, we want to print the
  external data format.
- Replace Parse with PARSE macro, apply check automatically.
- Update document, add the definition of str-str.

Signed-off-by: yoco <yoco@skymizer.com>
@yocox yocox force-pushed the feature/parser-support-external-data-format branch from 245260a to 0083c24 Compare October 22, 2023 14:33
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
@justinchuby justinchuby added this pull request to the merge queue Oct 25, 2023
Merged via the queue into onnx:main with commit 49d84c7 Oct 26, 2023
37 checks passed
@yocox yocox deleted the feature/parser-support-external-data-format branch October 28, 2023 04:15
isdanni pushed a commit to isdanni/onnx that referenced this pull request Nov 2, 2023
### Description
Improve the implementation of ONNX textual printer / parser to handle
external data format

### Motivation and Context
Recently the modern models get larger and larger. Models with external
data become common.
The ONNX textual format is good for human writing and reading. But is
dose not implement the
external data format, yet. This PR implement it.

---------

Signed-off-by: yoco <yoco@skymizer.com>
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Co-authored-by: yoco <yoco@skymizer.com>
Co-authored-by: Justin Chu <justinchuby@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants