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

[Relay][PASS] CombineParallelConv2D generating new kernel and causing performance issue #2827

Closed
kevinthesun opened this issue Mar 15, 2019 · 10 comments

Comments

@kevinthesun
Copy link
Contributor

Since some developers have the similar issue, I open this issue to track the status of discuss topic https://discuss.tvm.ai/t/relay-alter-op-layout-pass-regression/1870/7. CombineParallelConv2D pass generates new workloads and in some cases cause performance regression. Another issue is the combination of this pass with AutoTVM. This pass might need to be done before AutoTVM is launched.

@tqchen tqchen changed the title [Relay Pass Issue] CombineParallelConv2D generating new kernel and causing performance issue [Relay][PASS] CombineParallelConv2D generating new kernel and causing performance issue Mar 16, 2019
@yzhliu
Copy link
Member

yzhliu commented Mar 18, 2019

I think this pass can also be considered as a part of graph tuner.

@kevinthesun
Copy link
Contributor Author

@yzhliu Yes. Maybe we can add a call of this pass in autotvm before any other workload extraction/graph analysis is executed.

@tqchen
Copy link
Member

tqchen commented May 8, 2019

fixed by #2961

@tqchen tqchen closed this as completed May 8, 2019
@merrymercy
Copy link
Member

merrymercy commented May 18, 2019

Is this issue solved? We still don't call CombineParallelConv2d before workload extraction.
Reopen it.

@merrymercy merrymercy reopened this May 18, 2019
@vinx13
Copy link
Member

vinx13 commented May 18, 2019

CombineParallelConv2d doesn't always give better performance. We should extract workload with and without this pass and then decide whether to apply this pass.

@kevinthesun
Copy link
Contributor Author

Is it possible to move this pass to higher opt level? Looks like it's not quite suitable to put it together with other mandatory optimization passes in opt 3.

@vinx13
Copy link
Member

vinx13 commented May 20, 2019

@kevinthesun Yes we can move it to opt 4 for now

@vinx13
Copy link
Member

vinx13 commented May 21, 2019

Let me summarize the actions here:

  • Move CombineParallelConv2D to opt level 4
  • Extract autotvm tuning tasks: either require users to manually apply this pass before task extraction, or automatically extract tasks with & without this pass
  • Apply this pass if the performance is better, this can possibly be integrated with graph tuner

Would like to hear your comments @kevinthesun @merrymercy @yzhliu @tqchen

@kevinthesun
Copy link
Contributor Author

@vinx13 Sounds good to me. For autotvm and graph tuning parts, we can update the tutorial to add one more parameter to indicate whether to apply this pass before tuning.

@kevinthesun
Copy link
Contributor Author

CombineParallelConv2D moved to opt level 4. We can have another thread to deal with integration with autotvm.

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

No branches or pull requests

5 participants