Skip to content

Conversation

@jerryzh168
Copy link
Contributor

@jerryzh168 jerryzh168 commented Sep 12, 2019

Stack from ghstack:

Summary:
Add a QuantFusion to recursively fuse all the graphs that is invoked by
the method

Test Plan:
python test/test_jit.py 'TestJit.test_quant_fusion_module'

Reviewers:
pt1quant
Subscribers:

Tasks:

Tags:

Differential Revision: D17348985

Summary:
Add a QuantFusion to recursively fuse all the graphs that is invoked by
the method

Test Plan:
python test/test_jit.py 'TestJit.test_quant_fusion_module'

Reviewers:
pt1quant
Subscribers:

Tasks:

Tags:
@jerryzh168 jerryzh168 requested a review from apaszke as a code owner September 12, 2019 02:08
@pytorchbot pytorchbot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Sep 12, 2019
jerryzh168 added a commit that referenced this pull request Sep 12, 2019
Summary:
Add a QuantFusion to recursively fuse all the graphs that is invoked by
the method

Test Plan:
python test/test_jit.py 'TestJit.test_quant_fusion_module'

Reviewers:
pt1quant
Subscribers:

Tasks:

Tags:

ghstack-source-id: 1dbd956
Pull Request resolved: #26078
@ZolotukhinM
Copy link

In the end the fusion will happen in one of two places:

  • in graph executor along with other optimizations: at that point we won't have modules (and in fact we won't need them) - operating on a graph scope would work just fine as it is something we would execute.
  • in mobile workflow before serialization - in that case we want to run fusion in advance, but I think we would like to run them everywhere, not just in the method called by other methods - that would help us to use less operators in the end and potentially we could reduce the set of operators we need to deploy to mobile.

In neither of these scenarios we would be walking call-methods to find graphs to optimize, so I don't think this PR is the right approach.

@jerryzh168
Copy link
Contributor Author

it depends on the deployment flow, fusion is backend specific, let's say if we have some accelerators that can consume some form of jit graph with no jit runtime, then it is useful to fuse these graphs beforehand. I'm not sure what is the plan about graph executor and these fusion passes, should they always to coupled in the long run?

…lled"

Summary:
Add a QuantFusion to recursively fuse all the graphs that is invoked by
the method

Test Plan:
python test/test_jit.py 'TestJit.test_quant_fusion_module'

Reviewers:
pt1quant
Subscribers:

Tasks:

Tags:
jerryzh168 added a commit that referenced this pull request Sep 12, 2019
Summary:
Add a QuantFusion to recursively fuse all the graphs that is invoked by
the method

Test Plan:
python test/test_jit.py 'TestJit.test_quant_fusion_module'

Reviewers:
pt1quant
Subscribers:

Tasks:

Tags:

ghstack-source-id: f2c1c9b
Pull Request resolved: #26078
@jerryzh168
Copy link
Contributor Author

this is not needed since for fbgemm we'll integrate with graph executor, and for other backend we'll need to call QuantFusion(Graph) in the export flow. this as an independent function is not really used.

@jerryzh168 jerryzh168 closed this Sep 12, 2019
@ZolotukhinM
Copy link

it depends on the deployment flow, fusion is backend specific, let's say if we have some accelerators that can consume some form of jit graph with no jit runtime, then it is useful to fuse these graphs beforehand. I'm not sure what is the plan about graph executor and these fusion passes, should they always to coupled in the long run?

Fusion will always be a part of optimization pipeline - it could be either just-in-time optimization in graph executor, or ahead-of-time optimization for exporting to mobile/accelerator. In either case it's an always-valid transform and it will be run along with other general optimizations on all methods on all submodules. We don't need to invent the optimization pipeline ourselves here, having a function that optimizes on a graph scope should suffice now and in future.

@facebook-github-bot facebook-github-bot deleted the gh/jerryzh168/60/head branch October 28, 2019 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants