Skip to content

Conversation

@CircleSpin
Copy link
Contributor

This PR adds to two init files to make tvmc cleaner for running python scripts. For example, instead of running tvmc.runner.run_module() you can now run tvmc.run()

@CircleSpin
Copy link
Contributor Author

Copy link
Contributor

@mdw-octoml mdw-octoml left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@leandron leandron left a comment

Choose a reason for hiding this comment

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

Is the use case here that TVMC is used as an API within python scripts? it look OK to me, since it is part of TVM.

Can we think of a few test cases to add, to cover this new entry-point for tvmc as API?

cc @comaniac

@jwfromm
Copy link
Contributor

jwfromm commented Mar 19, 2021

@leandron yeah we think TVMC has an excellent set of utilities to make it a simplified python API for new users, kind of like keras. You did an awesome job packing tons of functionality into a few key function calls, but some of them need some small changes to better accommodate access from Python. @CircleSpin is working on making these changes.

@CircleSpin
Copy link
Contributor Author

I've modified the test files so that they use new the importing syntax. Does this look good to you @leandron ?

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

Copy link
Contributor

@leandron leandron left a comment

Choose a reason for hiding this comment

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

Thanks @CircleSpin

@jwfromm
Copy link
Contributor

jwfromm commented Mar 22, 2021

@merrymercy this PR is failing with this error message AttributeError: module 'tvm.auto_scheduler._ffi_api' has no attribute 'RandomModel', which almost certainly isn't caused by the changes made. Do you know what's going on?

@comaniac
Copy link
Contributor

Just a guess. Is that due to circular dependency?

@jwfromm jwfromm merged commit 1fe0abc into apache:main Mar 24, 2021
@jwfromm
Copy link
Contributor

jwfromm commented Mar 24, 2021

Thanks @CircleSpin @leandron and @comaniac. This is now merged.

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
* add to init files for clean tvmc python

* black reformat init.py

* adjust tests to new imports

* black test files

* tell lint ignore defined-builtin error for tvmc compile

* add colon to match lint syntax

* change import so must use tvm.driver.tvmc instead of tvm.tvmc

Co-authored-by: Jocelyn <[email protected]>
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request May 11, 2021
* add to init files for clean tvmc python

* black reformat init.py

* adjust tests to new imports

* black test files

* tell lint ignore defined-builtin error for tvmc compile

* add colon to match lint syntax

* change import so must use tvm.driver.tvmc instead of tvm.tvmc

Co-authored-by: Jocelyn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants