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.

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

[Runtime] Pipeline Executor Initial patch. #8702

Merged
merged 34 commits into from
Sep 15, 2021
Merged

[Runtime] Pipeline Executor Initial patch. #8702

merged 34 commits into from
Sep 15, 2021

Conversation

huajsj
Copy link
Contributor

@huajsj huajsj commented Aug 10, 2021

This patch is one of serial patch for PR 7892 splitting. this is initial part the pipeline executor, include the cmake
change and python and c++ interface for pipeline executor.

@jroesch
Copy link
Member

jroesch commented Aug 10, 2021

Is there an RFC for this feature? sorry if it was already posted some where, there is a lot of activity going on.

@huajsj
Copy link
Contributor Author

huajsj commented Aug 10, 2021

@jroesch , thanks for the follow up, here is the RFC apache/tvm-rfcs#14,
the discussion thread: https://discuss.tvm.apache.org/t/rfc-compute-graph-pipeline-with-new-subgraph-executor/9839/9

@huajsj
Copy link
Contributor Author

huajsj commented Aug 12, 2021

@comaniac

@comaniac
Copy link
Contributor

Sorry for the delay review. I'm busy with internal tasks now and will try my best to review the RFC again early next week.
We will be focusing on the RFC and start reviewing the PR with a consent.

@huajsj
Copy link
Contributor Author

huajsj commented Aug 12, 2021

thanks @comaniac.

@huajsj
Copy link
Contributor Author

huajsj commented Aug 27, 2021

@comaniac, the RFC design already applied into this PR, if you have time please review again.

comaniac
comaniac requested changes Aug 27, 2021
View reviewed changes
Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

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

Finish reviewing except for the detail implementation of PipelineModuleConfig. I would like to spend time on it after all the programming models and user interfaces are finalized.

CMakeLists.txt Outdated Show resolved Hide resolved
cmake/config.cmake Outdated Show resolved Hide resolved
python/tvm/contrib/pipeline_executor.py Outdated Show resolved Hide resolved
python/tvm/contrib/pipeline_executor.py Outdated Show resolved Hide resolved
python/tvm/contrib/pipeline_executor.py Outdated Show resolved Hide resolved
tests/python/relay/test_pipeline_executor.py Outdated Show resolved Hide resolved
tests/python/relay/test_pipeline_executor.py Outdated Show resolved Hide resolved
tests/python/relay/test_pipeline_executor.py Outdated Show resolved Hide resolved
tests/python/relay/test_pipeline_executor.py Outdated Show resolved Hide resolved
python/tvm/contrib/pipeline_executor.py Outdated Show resolved Hide resolved
@huajsj huajsj force-pushed the small branch 2 times, most recently from 2e24e76 to 6aa02a7 Compare August 29, 2021 20:36
comaniac
comaniac requested changes Aug 30, 2021
View reviewed changes
Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

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

Just carefully reviewed the pipeline config as it is the most important user interface of this feature. Please let me know if you have any concern.

python/tvm/contrib/pipeline_executor.py Outdated Show resolved Hide resolved
python/tvm/contrib/pipeline_executor.py Outdated Show resolved Hide resolved
python/tvm/contrib/pipeline_executor.py Outdated Show resolved Hide resolved
python/tvm/contrib/pipeline_executor.py Outdated Show resolved Hide resolved
python/tvm/contrib/pipeline_executor.py Outdated Show resolved Hide resolved
python/tvm/contrib/pipeline_executor.py Outdated Show resolved Hide resolved
python/tvm/contrib/pipeline_executor.py Outdated Show resolved Hide resolved
python/tvm/contrib/pipeline_executor.py Outdated Show resolved Hide resolved
python/tvm/contrib/pipeline_executor.py Outdated Show resolved Hide resolved
python/tvm/contrib/pipeline_executor.py Outdated Show resolved Hide resolved
@huajsj huajsj force-pushed the small branch 2 times, most recently from 95806b5 to 9cdf82b Compare September 1, 2021 04:23
@huajsj
Copy link
Contributor Author

huajsj commented Sep 2, 2021

@comaniac , all review comments addressed, please review again.

@masahi
Copy link
Member

masahi commented Sep 13, 2021

@huajsj See below. You said they were fixed, but they are not.

https://github.com/apache/tvm/pull/8702/files#r706562006
https://github.com/apache/tvm/pull/8702/files#r707197383
#8702 (comment)
#8702 (comment)

Before asking a review again, please go through ALL of your writing again and correct as much simple errors as possible (lack of a or the before a noun, s after third-pronoun verb, etc).

@huajsj
Copy link
Contributor Author

huajsj commented Sep 14, 2021

@masahi all review comments addressed, please review again.

@huajsj huajsj requested a review from masahi September 14, 2021 07:12
@masahi
Copy link
Member

masahi commented Sep 15, 2021

Thanks @huajsj for your patience. I left final comments, once they are fixed I'll merge this.

@huajsj
Copy link
Contributor Author

huajsj commented Sep 15, 2021

@masahi, Thanks for the kindly review, all review comments addressed, please take another look.

@masahi masahi merged commit 9bc4dc0 into apache:main Sep 15, 2021
@masahi
Copy link
Member

masahi commented Sep 15, 2021

thanks @huajsj @comaniac

AndrewZhaoLuo added a commit to AndrewZhaoLuo/tvm that referenced this pull request Sep 16, 2021
@AndrewZhaoLuo
Merge branch 'main' into aluo/onnx/sceloss
6d3aeb8 
* main: (102 commits)
  Implementation of relay_to_tir target hook (apache#8423)
  [Onnx] Fix NLL Loss tests (apache#8971)
  [Bugfix] Fix other div zero errors also in rewrite_simplify (apache#8983)
  [ONNX] enable the onnx tests after PR apache#8274 merged (apache#9019)
  [Hexagon] Disable `thread_local` on Hexagon (apache#9025)
  [Hexagon] Allow undefined symbols in libtvm_runtime.so on Hexagon (apache#9024)
  [Onnx] Add momentum (apache#9000)
  fix (apache#9021)
  [Community] @AndrewZhaoLuo -> Reviewer (apache#9020)
  [Hexagon] Implement model launcher (apache#8986)
  [Relay][Pass] Add ExtractOperators pass (apache#8996)
  [BYOC][TensorRT] Add TensorRT own int8 calibration support to TensorRT BYOC integration (apache#8808)
  [ONNX] Add Einsum converter (apache#8985)
  Add standalone_crt/ to be part of the wheel package, when available. (apache#9005)
  [Relay] Remove memory planing from LowerTEPass  (apache#8974)
  [Hexagon] Treat floats as float32 when passing args to offloaded kernels (apache#9010)
  [Runtime] Pipeline Executor Initial patch. (apache#8702)
  [Hexagon] `llvm-options` attribute is an array of strings (apache#9011)
  disable cuda int8 schedule for non-cuda gpu target (apache#9014)
  [Torch] Add an option to make imported models compatible with the Relay text parser (apache#9015)
  ...
ylc pushed a commit to ylc/tvm that referenced this pull request Sep 29, 2021
@huajsj @comaniac @ylc
[Runtime] Pipeline Executor Initial patch. (apache#8702)
c10ad96 
* [Runtime] Pipeline Executor Initial patch.

This patch is one of serial patch for PR 7892 splitting.this is the initial part
of the pipeline executor, this patch include the cmake change and python and C++
interface for pipeline executor.

* add pipeline config

* add config connect logic.

* fix build issue.

* set output index start from 0

* address review comments

Co-authored-by: Cody Yu <comaniac0422@gmail.com>

* address review comments.

* address review comments.

* Update python/tvm/contrib/pipeline_executor.py

Co-authored-by: Cody Yu <comaniac0422@gmail.com>

Update python/tvm/contrib/pipeline_executor.py

Co-authored-by: Cody Yu <comaniac0422@gmail.com>

* address review comments.

* address review comments

* add topology sort

* add binding check logic.

* fix plint error.

* Update python/tvm/contrib/pipeline_executor.py

Co-authored-by: Cody Yu <comaniac0422@gmail.com>

* address review comments.

* fix plint issue.

* address review comments.

* trigger build.

* Update python/tvm/contrib/pipeline_executor.py

Co-authored-by: Cody Yu <comaniac0422@gmail.com>

address review comments

* address review comments.

* polish doc and comments.

* polish doc and address review comments.

* address review comments.

* doc change.

* doc change.

* Trigger build.

* trigge build.

* address review comments.

* address review comments.

* address review comments.

* polish documents.

* Polish the document.

* address review comments.

Co-authored-by: Cody Yu <comaniac0422@gmail.com>
@junrushao junrushao mentioned this pull request Nov 1, 2021
Closed
@huajsj huajsj mentioned this pull request Nov 11, 2021
Closed
15 tasks
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
@huajsj @comaniac @ylc
[Runtime] Pipeline Executor Initial patch. (apache#8702)
96d6476 
* [Runtime] Pipeline Executor Initial patch.

This patch is one of serial patch for PR 7892 splitting.this is the initial part
of the pipeline executor, this patch include the cmake change and python and C++
interface for pipeline executor.

* add pipeline config

* add config connect logic.

* fix build issue.

* set output index start from 0

* address review comments

Co-authored-by: Cody Yu <comaniac0422@gmail.com>

* address review comments.

* address review comments.

* Update python/tvm/contrib/pipeline_executor.py

Co-authored-by: Cody Yu <comaniac0422@gmail.com>

Update python/tvm/contrib/pipeline_executor.py

Co-authored-by: Cody Yu <comaniac0422@gmail.com>

* address review comments.

* address review comments

* add topology sort

* add binding check logic.

* fix plint error.

* Update python/tvm/contrib/pipeline_executor.py

Co-authored-by: Cody Yu <comaniac0422@gmail.com>

* address review comments.

* fix plint issue.

* address review comments.

* trigger build.

* Update python/tvm/contrib/pipeline_executor.py

Co-authored-by: Cody Yu <comaniac0422@gmail.com>

address review comments

* address review comments.

* polish doc and comments.

* polish doc and address review comments.

* address review comments.

* doc change.

* doc change.

* Trigger build.

* trigge build.

* address review comments.

* address review comments.

* address review comments.

* polish documents.

* Polish the document.

* address review comments.

Co-authored-by: Cody Yu <comaniac0422@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@masahi masahi masahi approved these changes

@comaniac comaniac comaniac approved these changes

@areusch areusch Awaiting requested review from areusch

@jroesch jroesch Awaiting requested review from jroesch

@junrushao junrushao Awaiting requested review from junrushao

@kazum kazum Awaiting requested review from kazum

@liangfu liangfu Awaiting requested review from liangfu

@merrymercy merrymercy Awaiting requested review from merrymercy

@tmoreau89 tmoreau89 Awaiting requested review from tmoreau89

@tqchen tqchen Awaiting requested review from tqchen

@vinx13 vinx13 Awaiting requested review from vinx13

@yzhliu yzhliu Awaiting requested review from yzhliu

@zhiics zhiics Awaiting requested review from zhiics

@ZihengJiang ZihengJiang Awaiting requested review from ZihengJiang

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@huajsj @jroesch @comaniac @masahi