Skip to content

[ONNX] Improve error checking for large model export#37798

Closed
BowenBao wants to merge 1 commit intopytorch:masterfrom
BowenBao:onnx_large_model_error
Closed

[ONNX] Improve error checking for large model export#37798
BowenBao wants to merge 1 commit intopytorch:masterfrom
BowenBao:onnx_large_model_error

Conversation

@BowenBao
Copy link
Copy Markdown
Collaborator

@BowenBao BowenBao commented May 4, 2020

  • Add error message when onnx model file path is not a string.
  • Add error message when model size exceed 2GB when large model export is not turned on.

@BowenBao BowenBao requested a review from apaszke as a code owner May 4, 2020 22:17
@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label May 4, 2020
@ngimel ngimel requested a review from houseroad May 5, 2020 00:33
@ngimel ngimel added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label May 5, 2020
Copy link
Copy Markdown
Contributor

@neginraoof neginraoof left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

add_node_names,
use_external_data_format,
onnx_file_path);
const size_t proto_size = graph_encoder.get_model_proto().ByteSizeLong();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am wondering whether model_proto will be larger than 2GB, or just directly truncated when dumping to string.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nvm, not a valid question

Copy link
Copy Markdown
Member

@houseroad houseroad left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks.

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@houseroad merged this pull request in 122587d.

laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
Summary:
* Add error message when onnx model file path is not a string.
* Add error message when model size exceed 2GB when large model export is not turned on.
Pull Request resolved: pytorch#37798

Reviewed By: hl475

Differential Revision: D21440571

Pulled By: houseroad

fbshipit-source-id: 054aaa25ab0cffc229f9b487a2c160623c89b741
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged oncall: jit Add this issue/PR to JIT oncall triage queue open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants