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

[Lang] Layout in TVM node system #2509

Merged
merged 21 commits into from
Feb 28, 2019
Merged

Conversation

yzhliu
Copy link
Member

@yzhliu yzhliu commented Jan 25, 2019

Per RFC we introduce Layout into TVM node system, and remove that from Relay.

Reviewers Please help to review @kevinthesun @merrymercy @tqchen

@tqchen
Copy link
Member

tqchen commented Jan 25, 2019

@yzhliu do you mind summarize the discuss thread a bit and open an formal RFC issue?

@yzhliu
Copy link
Member Author

yzhliu commented Jan 25, 2019

@tqchen sure, will do.

@merrymercy merrymercy self-assigned this Jan 28, 2019
@kevinthesun
Copy link
Contributor

I'll start to review after RFC is out.

@merrymercy
Copy link
Member

Only some minors, generally looks good to me

@kevinthesun
Copy link
Contributor

LGTM as long as we add topi layout_transform.

@merrymercy merrymercy added status: need update need update based on feedbacks and removed status: need review labels Feb 24, 2019
Copy link
Member

@tqchen tqchen left a comment

Choose a reason for hiding this comment

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

Some quick feedbacks, overall looks good

@yzhliu
Copy link
Member Author

yzhliu commented Feb 25, 2019

@kevinthesun @merrymercy @tqchen Thanks for feedback. Comments addressed, please review again.

* \brief Layout expression to describe the data organization of a tensor.
* And BijectiveLayout to mapping two data layouts between each other.
*/
#ifndef TVM_LAYOUT_H_
Copy link
Member

Choose a reason for hiding this comment

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

How about we rename layout.h ->data_layout just to make the header name clear, we do not have to rename anything in the class

@tqchen
Copy link
Member

tqchen commented Feb 27, 2019

Have one comment on changing layout.h->data_layout.h otherwise lgtm, will leave it to @merrymercy for moderation

@yzhliu
Copy link
Member Author

yzhliu commented Feb 27, 2019

@tqchen @merrymercy @kevinthesun Comment addressed. Please check again.

@merrymercy merrymercy merged commit ee79703 into apache:master Feb 28, 2019
@merrymercy
Copy link
Member

@tqchen @yzhliu @kevinthesun Thanks. It is merged

@yzhliu yzhliu mentioned this pull request Mar 2, 2019
28 tasks
bwasti pushed a commit to facebookexperimental/tvm that referenced this pull request Mar 6, 2019
* move layout.h & layout.cc from relay to tvm

* change ConvertLayout in relay to bijectiveLayout->Forward/backward

* add first test case

* add LayoutAxis

* add LayoutAxis struct and compiles

* simplify BijectiveLayout rule consturct

* polish func name for Layout, move impl to .cc, remove Layout::defined(), add defined() checker

* partially add layout py support

* add layout test cases

* add doc for tvm.layout & tvm.bijective_layout

* fix lint

* fix lint

* fix layout name generation bug

* fix layout typo

* address comments and add topi.layout_transform

* layout.h->data_layout.h, test_lang_layout.py->test_lang_data_layout.py
wweic pushed a commit to neo-ai/tvm that referenced this pull request Mar 9, 2019
* move layout.h & layout.cc from relay to tvm

* change ConvertLayout in relay to bijectiveLayout->Forward/backward

* add first test case

* add LayoutAxis

* add LayoutAxis struct and compiles

* simplify BijectiveLayout rule consturct

* polish func name for Layout, move impl to .cc, remove Layout::defined(), add defined() checker

* partially add layout py support

* add layout test cases

* add doc for tvm.layout & tvm.bijective_layout

* fix lint

* fix lint

* fix layout name generation bug

* fix layout typo

* address comments and add topi.layout_transform

* layout.h->data_layout.h, test_lang_layout.py->test_lang_data_layout.py
wweic pushed a commit to neo-ai/tvm that referenced this pull request Mar 12, 2019
* move layout.h & layout.cc from relay to tvm

* change ConvertLayout in relay to bijectiveLayout->Forward/backward

* add first test case

* add LayoutAxis

* add LayoutAxis struct and compiles

* simplify BijectiveLayout rule consturct

* polish func name for Layout, move impl to .cc, remove Layout::defined(), add defined() checker

* partially add layout py support

* add layout test cases

* add doc for tvm.layout & tvm.bijective_layout

* fix lint

* fix lint

* fix layout name generation bug

* fix layout typo

* address comments and add topi.layout_transform

* layout.h->data_layout.h, test_lang_layout.py->test_lang_data_layout.py
wweic pushed a commit to neo-ai/tvm that referenced this pull request Mar 12, 2019
* move layout.h & layout.cc from relay to tvm

* change ConvertLayout in relay to bijectiveLayout->Forward/backward

* add first test case

* add LayoutAxis

* add LayoutAxis struct and compiles

* simplify BijectiveLayout rule consturct

* polish func name for Layout, move impl to .cc, remove Layout::defined(), add defined() checker

* partially add layout py support

* add layout test cases

* add doc for tvm.layout & tvm.bijective_layout

* fix lint

* fix lint

* fix layout name generation bug

* fix layout typo

* address comments and add topi.layout_transform

* layout.h->data_layout.h, test_lang_layout.py->test_lang_data_layout.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: need update need update based on feedbacks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants