Skip to content

ONNX: fix default function value in _optimize_graph #83996

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

ONNX: fix default function value in _optimize_graph #83996

wants to merge 4 commits into from

Conversation

ThibaultCastells
Copy link

The default value for params_dict in _optimize_graph, which is None, throw the following error:

_C._jit_pass_onnx_unpack_quantized_weights(

TypeError: _jit_pass_onnx_unpack_quantized_weights(): incompatible function arguments. The following argument types are supported:
1. (arg0: torch::jit::Graph, arg1: Dict[str, IValue], arg2: bool) -> Dict[str, IValue]

Replacing it by an empty dict fixes the issue (and makes more sense).

@facebook-github-bot
Copy link
Contributor

Hi @ThibaultCastells!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Aug 24, 2022 •
edited
Loading

🔗 Helpful links

✅ No Failures (0 Pending)

As of commit a7a9989 (more details on the Dr. CI page):

Expand to see more

💚 💚 Looks good so far! There are no failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

BowenBao
BowenBao reviewed Aug 24, 2022
View reviewed changes
@BowenBao
Copy link
Collaborator

Thanks for contributing! Please see inline comment. Also please be sure to sign contributor license agreement.

@BowenBao BowenBao self-assigned this Aug 24, 2022
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@BowenBao BowenBao added module: onnx Related to torch.onnx onnx-triaged triaged by ONNX team release notes: onnx torch.onnx related changes that should show up in the release notes topic: improvements topic category and removed cla signed labels Aug 24, 2022
@ngimel ngimel added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Aug 24, 2022
@ThibaultCastells
Copy link
Author

@BowenBao Thanks for reviewing! I updated the commit according to your comment, and signed the CLA

@BowenBao
Copy link
Collaborator

@pytorchbot merge -g

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a merge job. Check the current status here.
The merge job was triggered with the green (-g) flag. This means that your change will be merged once all checks on your PR have passed (ETA: 0-4 Hours). If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

@pytorchmergebot
Copy link
Collaborator

Merge failed
Reason: View failures on hud. Refusing to merge as mandatory check(s) Lint failed for rule ONNX exporter.
Raised by workflow job

@BowenBao
Copy link
Collaborator

@pytorchbot rebase

@pytorch-bot
Copy link

pytorch-bot bot commented Aug 25, 2022

You don't have permissions to rebase this PR, only the PR author and pytorch organization members may rebase this PR.

@BowenBao
Copy link
Collaborator

@ThibaultCastells The PR is ready to merge as soon as all CI passing. Could you please do a rebase with latest master to resolve all CI issues?

@ThibaultCastells
Copy link
Author

ThibaultCastells commented Aug 28, 2022 •
edited
Loading

@BowenBao I rebased, it looks like they all passed

BowenBao
BowenBao reviewed Aug 30, 2022
View reviewed changes
BowenBao
BowenBao reviewed Aug 30, 2022
View reviewed changes
BowenBao
BowenBao reviewed Aug 30, 2022
View reviewed changes
BowenBao
BowenBao reviewed Sep 2, 2022
View reviewed changes
BowenBao
BowenBao reviewed Sep 2, 2022
View reviewed changes
@ThibaultCastells
Copy link
Author

@BowenBao Still not successful... Should I recreate the pull request from scratch?

@BowenBao
Copy link
Collaborator

BowenBao commented Sep 6, 2022

@BowenBao Still not successful... Should I recreate the pull request from scratch?

Let's give rebasing one last try, could you comment with @pytorchbot rebase?

@ThibaultCastells
Copy link
Author

@pytorchbot rebase

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a rebase job. Check the current status here

@pytorchmergebot
Copy link
Collaborator

Successfully rebased onnx_fix onto refs/remotes/origin/master, please pull locally before adding more changes (for example, via git checkout onnx_fix && git pull --rebase)

@BowenBao
Copy link
Collaborator

BowenBao commented Sep 6, 2022

@pytorchbot merge -g

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a merge job. Check the current status here.
The merge job was triggered with the green (-g) flag. This means that your change will be merged once all checks on your PR have passed (ETA: 0-4 Hours). If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

facebook-github-bot pushed a commit that referenced this pull request Sep 8, 2022
@facebook-github-bot
ONNX: fix default function value in _optimize_graph (#83996) (#83996)
108f947 
Summary:
The default value for params_dict in _optimize_graph, which is None, throw the following error:

>     _C._jit_pass_onnx_unpack_quantized_weights(
> TypeError: _jit_pass_onnx_unpack_quantized_weights(): incompatible function arguments. The following argument types are supported:
>     1. (arg0: torch::jit::Graph, arg1: Dict[str, IValue], arg2: bool) -> Dict[str, IValue]

Replacing it by an empty dict fixes the issue (and makes more sense).

Pull Request resolved: #83996
Approved by: https://github.com/BowenBao

Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/d9ceda49c497bc39c2b360b038ee07e145b32f5b

Reviewed By: izaitsevfb

Differential Revision: D39297070

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

@BowenBao BowenBao BowenBao approved these changes

Assignees

@BowenBao BowenBao

Labels
cla signed Merged module: onnx Related to torch.onnx onnx-triaged triaged by ONNX team open source release notes: onnx torch.onnx related changes that should show up in the release notes topic: improvements topic category triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ThibaultCastells @facebook-github-bot @BowenBao @pytorchmergebot @ngimel @pytorchbot