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

[RELAY] IR builder stablize refactor, clean pass #1934

Merged
merged 2 commits into from
Oct 20, 2018
Merged

[RELAY] IR builder stablize refactor, clean pass #1934

merged 2 commits into from
Oct 20, 2018

Conversation

tqchen
Copy link
Member

@tqchen tqchen commented Oct 19, 2018 •
edited
Loading

This is a clean refactor PR to attempt stablize the relay python IR building API, it also contains fixes and patches to enable text format.

One of the general guidelines behind this refactor are:

  • Minimum abstraction when possible (simplification of IRBuilder)
  • Python API consistency (env.merge->env.update)
  • Make ir_pass style consistent
    • Expr is always the first argument
    • Not require env to be present, when necessary, pass as a second argument

List of Changes

  • Simplify ir_builder to scope_builder cc @jroesch
    • ScopeBuilder is only used to build if and let scopes.
    • Use relay.Function to build functions (single way to build functions)
  • Rewrite all the test cases with the new style.
  • Add text print test to each op, fix problems that surface due to the new testcases.
  • Improve the BroadcastRel to handle symbolic case cc @jroesch
  • Fix TupleWrapper cc @slyubomirsky
    • To make sure python unpack works, we need to throw index error when there is out of bound access
    • TupleWrapper cannot assume the input field is a tuple type
  • Enable TulpleGetItem type inference with incomplete type
  • Fix dead code elimination when type is bool cc @MarisaKirisame
  • Fix LeakyReluAttrs cc @siju-samuel
  • Merge reduce tests into a more concise functioncc @siju-samuel
  • Rename TypeParam->TypeVar as suggested by @joshpoll

@tqchen
Copy link
Member Author

tqchen commented Oct 19, 2018

@jroesch @MarisaKirisame @junrushao1994 @slyubomirsky please review

@junrushao
Copy link
Member

Will do tonight

MarisaKirisame
MarisaKirisame reviewed Oct 19, 2018
View reviewed changes
src/relay/ir/environment.cc Outdated
this->global_map_.Set(str, id);
return id;
}
GlobalVar EnvironmentNode::GetGlobalVar(const std::string &name) {
Copy link
Contributor

Choose a reason for hiding this comment

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

const std::string& name

src/relay/ir/environment.cc Outdated
CHECK(type.as<IncompleteTypeNode>() == nullptr);
if (functions.find(var) != functions.end()) {
if (!update) {
throw dmlc::Error("already have definition for XXXX.");
Copy link
Contributor

Choose a reason for hiding this comment

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

do XXXX

@tqchen tqchen merged commit 8876eac into apache:master Oct 20, 2018
This was referenced Oct 20, 2018
Closed
Merged
Merged
Merged
Merged
FrozenGene pushed a commit to FrozenGene/tvm that referenced this pull request Dec 27, 2018
@tqchen
[RELAY] IR builder stablize refactor, clean pass (apache#1934)
e15e2e3
wweic pushed a commit to neo-ai/tvm that referenced this pull request Feb 20, 2019
@tqchen
[RELAY] IR builder stablize refactor, clean pass (apache#1934)
7877514
wweic pushed a commit to neo-ai/tvm that referenced this pull request Feb 20, 2019
@tqchen
[RELAY] IR builder stablize refactor, clean pass (apache#1934)
06f14a7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarisaKirisame MarisaKirisame MarisaKirisame left review comments

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

Successfully merging this pull request may close these issues.
3 participants
@tqchen @junrushao @MarisaKirisame