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

[ONNX] Update logic for folding onnx::Constant nodes. #20109

Conversation

spandantiwari
Copy link

Currently, constant folding pass during ONNX conversion removes all onnx::Constant nodes that are parents of nodes that are folded. In situations where the parent onnx::Constant node is other subscribers downstream this could be a problem. This change updates the removal logic to remove to only those onnx::Constant nodes that do not have other subscribers downstream

@pytorchbot pytorchbot added oncall: jit Add this issue/PR to JIT oncall triage queue module: onnx Related to torch.onnx labels May 3, 2019
@spandantiwari
Copy link
Author

cc: @houseroad

@spandantiwari
Copy link
Author

@houseroad - The failure doesn't seem related to this change.

@spandantiwari
Copy link
Author

@houseroad - It passed on rerun now.

@ezyang ezyang requested a review from houseroad May 6, 2019 14:39
Copy link
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.

Actually, we can add a constant fusion pass in onnx exporter. So the constants with the same value can be merged. :-)

Copy link
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.

@spandantiwari
Copy link
Author

@houseroad - that sounds like a good idea.

@facebook-github-bot
Copy link
Contributor

@houseroad merged this pull request in 838ada3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merged module: onnx Related to torch.onnx oncall: jit Add this issue/PR to JIT oncall triage queue open source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants