Skip to content

[RELAY] [AST] Add virtual_device as a first class field in Relay#9641

Merged
tqchen merged 5 commits intoapache:mainfrom
electriclilies:first-class-scope
Dec 10, 2021
Merged

[RELAY] [AST] Add virtual_device as a first class field in Relay#9641
tqchen merged 5 commits intoapache:mainfrom
electriclilies:first-class-scope

Conversation

@electriclilies
Copy link
Copy Markdown
Contributor

This PR adds virtual_device (SEScope) as a first class field in Relay. This is an implementation of apache/tvm-rfcs#45 please take a look the RFC for the motivation, implementation, future work, etc.

@mbs-octoml @jroesch @mikepapadim

@electriclilies electriclilies changed the title Add virtual_device as a first class field in Relay [RELAY] [AST] Add virtual_device as a first class field in Relay Dec 2, 2021
Copy link
Copy Markdown
Contributor

@mbs-octoml mbs-octoml left a comment

Choose a reason for hiding this comment

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

Thanks Lily, this can't come soon enough as I spent yesterday tracking down more accidentally-lost on_device annotations!

Comment thread include/tvm/relay/expr.h Outdated
@electriclilies
Copy link
Copy Markdown
Contributor Author

@comaniac Could you take a look at this? It's my implementation of apache/tvm-rfcs#45. Thanks!

Copy link
Copy Markdown
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.

Overall LGTM. Just some docstring errors.

Comment thread include/tvm/ir/expr.h Outdated
Comment thread include/tvm/relay/expr.h Outdated
Comment thread include/tvm/relay/expr.h Outdated
Comment thread include/tvm/relay/expr.h Outdated
Comment thread include/tvm/relay/expr.h
Comment thread include/tvm/relay/expr.h Outdated
Comment thread include/tvm/relay/expr.h Outdated
@electriclilies electriclilies force-pushed the first-class-scope branch 2 times, most recently from fa8466d to 1637380 Compare December 8, 2021 21:41
@electriclilies
Copy link
Copy Markdown
Contributor Author

electriclilies commented Dec 8, 2021

@comaniac Thanks, updated the comments and docstrings! I think this is good to go in once CI is green.

Copy link
Copy Markdown
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.

LGTM

@tqchen tqchen merged commit bd361b9 into apache:main Dec 10, 2021
@tqchen
Copy link
Copy Markdown
Member

tqchen commented Dec 10, 2021

THanks @electriclilies for the contribution. Thanks @comaniac @mbs-octoml for reviewing. This PR is now merged

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.

4 participants