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] A Normal Form Canonicalization #2251

Merged
merged 5 commits into from
Jan 24, 2019
Merged

Conversation

MarisaKirisame
Copy link
Contributor

this pr convert any code in graph_normal_form (has sharing in internal AST) into a_normal_form.

@MarisaKirisame
Copy link
Contributor Author

@jroesch is this what you need?

@MarisaKirisame
Copy link
Contributor Author

@siju-samuel @vinx13 @srkreddy1238 please review.

@tqchen
Copy link
Member

tqchen commented Dec 7, 2018

@MarisaKirisame Given that there are many possible ways to convert Graph to ANF, can you first send an RFC to propose the "normal form" of conversion? We'd better document it properly.

Copy link
Member

@vinx13 vinx13 left a comment

Choose a reason for hiding this comment

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

can you add some tests?

src/relay/pass/to_anf.cc Outdated Show resolved Hide resolved
src/relay/pass/let_list.h Outdated Show resolved Hide resolved
src/relay/pass/to_anf.cc Outdated Show resolved Hide resolved
src/relay/pass/to_anf.cc Outdated Show resolved Hide resolved
@icemelon icemelon added the status: need update need update based on feedbacks label Dec 21, 2018
@MarisaKirisame
Copy link
Contributor Author

@tqchen I have address the review comment and reply to the rfc, can you give a round of review? (the rfc is #2253 for all that dont know)

@tqchen tqchen changed the title [Relay] to_anf [Relay] A Normal Form Canonicalization Dec 31, 2018
@MarisaKirisame
Copy link
Contributor Author

@vinx13 @srkreddy1238 @tqchen can you guys give one round of review? i had resolved the issues and added more test.

src/relay/pass/to_anf.cc Outdated Show resolved Hide resolved
src/relay/pass/to_anf.cc Outdated Show resolved Hide resolved
src/relay/pass/to_anf.cc Outdated Show resolved Hide resolved
include/tvm/relay/pass.h Outdated Show resolved Hide resolved
src/relay/pass/to_anf.cc Outdated Show resolved Hide resolved
src/relay/pass/to_anf.cc Outdated Show resolved Hide resolved
include/tvm/relay/pass.h Outdated Show resolved Hide resolved
include/tvm/relay/pass.h Outdated Show resolved Hide resolved
src/relay/pass/to_anf.cc Show resolved Hide resolved
src/relay/pass/to_anf.cc Outdated Show resolved Hide resolved
src/relay/pass/to_anf.cc Outdated Show resolved Hide resolved
src/relay/pass/to_anf.cc Show resolved Hide resolved
src/relay/pass/to_anf.cc Outdated Show resolved Hide resolved
src/relay/pass/to_anf.cc Outdated Show resolved Hide resolved
include/tvm/relay/pass.h Outdated Show resolved Hide resolved
include/tvm/relay/pass.h Outdated Show resolved Hide resolved
src/relay/pass/fuse_ops.cc Outdated Show resolved Hide resolved
src/relay/pass/to_anf.cc Outdated Show resolved Hide resolved
src/relay/pass/to_anf.cc Show resolved Hide resolved
src/relay/pass/to_anf.cc Show resolved Hide resolved
src/relay/pass/to_anf.cc Outdated Show resolved Hide resolved
@MarisaKirisame MarisaKirisame force-pushed the to_anf branch 2 times, most recently from f9e8665 to 028f49a Compare January 12, 2019 05:00
save

do it
@MarisaKirisame
Copy link
Contributor Author

@ZihengJiang can you review?

*
* \return expression in A Normal Form
*/
Expr ToANF(const Expr& e, const Module& mod);
Copy link
Member

Choose a reason for hiding this comment

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

Let use use ToANormalForm, so everyone can google and find out what is ANF

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about the python side? It wa to_anf.

@tqchen
Copy link
Member

tqchen commented Jan 18, 2019

@junrushao1994 @ZihengJiang can you help review this PR?

@junrushao
Copy link
Member

Will do tonight

Copy link
Member

@junrushao junrushao left a comment

Choose a reason for hiding this comment

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

Looks good.

include/tvm/relay/pass.h Outdated Show resolved Hide resolved
include/tvm/relay/pass.h Outdated Show resolved Hide resolved
include/tvm/relay/pass.h Outdated Show resolved Hide resolved
src/relay/pass/let_list.h Show resolved Hide resolved
junrushao and others added 3 commits January 19, 2019 10:14
@tqchen tqchen merged commit 12aca82 into apache:master Jan 24, 2019
@tqchen
Copy link
Member

tqchen commented Jan 24, 2019

Thanks, @MarisaKirisame @junrushao1994 ! this is now merged

@tqchen tqchen added status: accepted and removed status: need update need update based on feedbacks labels Jan 24, 2019
@MarisaKirisame MarisaKirisame deleted the to_anf branch January 24, 2019 21:49
Anthony-Mai pushed a commit to Anthony-Mai/tvm that referenced this pull request Jan 25, 2019
zhiics pushed a commit to zhiics/tvm that referenced this pull request Jan 30, 2019
@ZihengJiang ZihengJiang mentioned this pull request Feb 1, 2019
merrymercy pushed a commit to merrymercy/tvm that referenced this pull request Feb 18, 2019
wweic pushed a commit to neo-ai/tvm that referenced this pull request Feb 20, 2019
wweic pushed a commit to neo-ai/tvm that referenced this pull request Feb 20, 2019
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.

5 participants