Skip to content

Conversation

@electriclilies
Copy link
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
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!

@electriclilies
Copy link
Contributor Author

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

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

@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
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
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
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