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

[docs] Add a tutorial for the pass manager #3515

Merged
merged 14 commits into from
Jul 17, 2019
Merged

Conversation

zhiics
Copy link
Member

@zhiics zhiics commented Jul 9, 2019

#3480

This PR adds a tutorial for the recently merged pass manager infra.

cc @tqchen @jroesch @weberlo @slyubomirsky @wweic @yongwww @yzhliu @icemelon9 @MarisaKirisame

docs/dev/relay_pass_manager.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_manager.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_manager.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_manager.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_manager.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_manager.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_manager.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_manager.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_manager.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_manager.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_manager.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_manager.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_manager.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_manager.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_manager.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_manager.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_manager.rst Outdated Show resolved Hide resolved
@weberlo
Copy link
Contributor

weberlo commented Jul 9, 2019

Awesome work! This tutorial seems very comprehensive and has already helped me understand the pass manager a lot more. I'll do another round of review at some point.

docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Show resolved Hide resolved
Copy link
Contributor

@slyubomirsky slyubomirsky left a comment

Choose a reason for hiding this comment

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

Mostly some stylistic and grammar suggestions, and a couple of requests for clarification. Overall, I think this is a fairly thorough explanation of the pass manager and I am glad to see the extensive examples -- thank you very much for taking the time to write it all

docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Show resolved Hide resolved
@yzhliu yzhliu added the status: need update need update based on feedbacks label Jul 11, 2019
@zhiics
Copy link
Member Author

zhiics commented Jul 12, 2019

@anijain2305 @weberlo @shoubhik @slyubomirsky @merrymercy Thanks for the reviews. I've addressed your comments. Please take another look when you have time.

docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@slyubomirsky slyubomirsky left a comment

Choose a reason for hiding this comment

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

I think the document is in a good state. The below comments are about small typos or grammatical nitpicks, but overall I think the document is very good

docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
zhiics and others added 5 commits July 15, 2019 18:23
Co-Authored-By: Steven S. Lyubomirsky <[email protected]>
Co-Authored-By: Steven S. Lyubomirsky <[email protected]>
Co-Authored-By: Steven S. Lyubomirsky <[email protected]>
Co-Authored-By: Steven S. Lyubomirsky <[email protected]>
Co-Authored-By: Steven S. Lyubomirsky <[email protected]>
@zhiics
Copy link
Member Author

zhiics commented Jul 16, 2019

@slyubomirsky Thanks a lot.

@weberlo @shoubhik @anijain2305 @merrymercy Thank you all for the review. Please let me if you have any other comments or concerns.

docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
docs/dev/relay_pass_infra.rst Outdated Show resolved Hide resolved
@yzhliu yzhliu merged commit ce363d6 into apache:master Jul 17, 2019
@yzhliu
Copy link
Member

yzhliu commented Jul 17, 2019

Thanks everyone, this is now merged.

@yzhliu yzhliu added status: accepted and removed status: need update need update based on feedbacks labels Jul 17, 2019
@zhiics zhiics deleted the pass_doc branch July 17, 2019 22:14
wweic pushed a commit to wweic/tvm that referenced this pull request Aug 9, 2019
* [docs] Add a tutorial for the pass manager

* address comment

* address more comments

* retrigger ci

* address steven's comments

* address comments

* retrigger ci

* Update docs/dev/relay_pass_infra.rst

Co-Authored-By: Steven S. Lyubomirsky <[email protected]>

* Update docs/dev/relay_pass_infra.rst

Co-Authored-By: Steven S. Lyubomirsky <[email protected]>

* Update docs/dev/relay_pass_infra.rst

Co-Authored-By: Steven S. Lyubomirsky <[email protected]>

* Update docs/dev/relay_pass_infra.rst

Co-Authored-By: Steven S. Lyubomirsky <[email protected]>

* Update docs/dev/relay_pass_infra.rst

Co-Authored-By: Steven S. Lyubomirsky <[email protected]>

* Update docs/dev/relay_pass_infra.rst

Co-Authored-By: Logan Weber <[email protected]>

* Update docs/dev/relay_pass_infra.rst

Co-Authored-By: Logan Weber <[email protected]>
wweic pushed a commit to neo-ai/tvm that referenced this pull request Sep 6, 2019
* [docs] Add a tutorial for the pass manager

* address comment

* address more comments

* retrigger ci

* address steven's comments

* address comments

* retrigger ci

* Update docs/dev/relay_pass_infra.rst

Co-Authored-By: Steven S. Lyubomirsky <[email protected]>

* Update docs/dev/relay_pass_infra.rst

Co-Authored-By: Steven S. Lyubomirsky <[email protected]>

* Update docs/dev/relay_pass_infra.rst

Co-Authored-By: Steven S. Lyubomirsky <[email protected]>

* Update docs/dev/relay_pass_infra.rst

Co-Authored-By: Steven S. Lyubomirsky <[email protected]>

* Update docs/dev/relay_pass_infra.rst

Co-Authored-By: Steven S. Lyubomirsky <[email protected]>

* Update docs/dev/relay_pass_infra.rst

Co-Authored-By: Logan Weber <[email protected]>

* Update docs/dev/relay_pass_infra.rst

Co-Authored-By: Logan Weber <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants