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

[microTVM] Add support for AutoTVM #8715

Merged
merged 78 commits into from
Sep 9, 2021

Conversation

mehrdadh
Copy link
Member

This PR fixes broken auto tune for microTVM targets using project api.

cc @areusch

 * use template crt_config.h for host test runtime; delete
   src/runtime/crt/host/crt_config.h so that it doesn't diverge from
   the template
 * bring template crt_config.h inline with the one actually in use
  * rename to MAX_STRLEN_DLTYPE
 * Create a dedicated TVM-side host crt_config.h in src/runtime/micro
 * move all zephyr projects to apps/microtvm/zephyr/template_project
 * Allows tracker clients to open non-traditional RPC sessions
@mehrdadh
Copy link
Member Author

@tkonolige thanks for you review! I think I answered most comments, the only thing left is the global RPC_SESSION that I have to figure out. PTAL.
Thanks!

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.

It looks generally good. A few nits below.

python/tvm/micro/build.py Outdated Show resolved Hide resolved
tutorials/micro/micro_autotune.py Outdated Show resolved Hide resolved
tutorials/micro/micro_autotune.py Outdated Show resolved Hide resolved
python/tvm/autotvm/measure/measure_methods.py Outdated Show resolved Hide resolved
@mehrdadh
Copy link
Member Author

@leandron thanks for the review. I think I addressed your concerns. PTAL.

python/tvm/support.py Outdated Show resolved Hide resolved
python/tvm/support.py Outdated Show resolved Hide resolved
python/tvm/__init__.py Outdated Show resolved Hide resolved
Comment on lines 296 to 304
@register_func
def destroy_micro_session():
global RPC_SESSION
if RPC_SESSION is not None:
exc_type, exc_value, traceback = RPC_SESSION.__exit__(None, None, None)
RPC_SESSION = None
if (exc_type, exc_value, traceback) != (None, None, None):
exc = exc_type(exc_value) # See PEP 3109
exc.__traceback__ = traceback
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this called? I can't seem to find another reference to it in this PR or the codebase.

Copy link
Contributor

Choose a reason for hiding this comment

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

good catch @tkonolige . so at present, the return value from session_constructor is not captured into RPCEndpoint. This means we cannot unfortunately rely on del hook from RPCEndpoint getting destructed in order to destroy any Python-side resources. I suggest then to avoid blocking this PR, we add a call (and only log exceptions, not throw them--in case the link breaks) to destroy_micro_session after the yield:

try:
  remote.get_function("tvm.micro.destroy_micro_session")()
except TVMError:
  _LOG.warning("Error destroying remote session", exc_info=1)

Copy link
Contributor

Choose a reason for hiding this comment

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

another option is to capture the return value as ObjectRef or TVMValue in RPCEndpoint and then either have the return value be a PackedFunc which can be invoked at RPCEndpoint disconnect time (but that sucks due to destructor-may-throw-exception) or an ObjectRef which we del. I prefer the former; but the former kind of is asking for us to ensure Shutdown() gets called before ~RPCEndpoint.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can do the call for now. A separate PR for handling session lifetimes would be good to have.

Copy link
Member Author

Choose a reason for hiding this comment

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

@areusch thanks for the suggestion. I added that.

tutorials/micro/micro_autotune.py Outdated Show resolved Hide resolved
tutorials/micro/micro_autotune.py Outdated Show resolved Hide resolved
tutorials/micro/micro_autotune.py Outdated Show resolved Hide resolved
Copy link
Contributor

@tkonolige tkonolige 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 now. Thanks for all the hard work (and many commits :) )!

Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

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

thanks @mehrdadh !

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.

LGTM. I'm happy for this to be merged once CI is green.

@mehrdadh
Copy link
Member Author

@leandron Thanks for the review. Does this need a rebase?

@leandron
Copy link
Contributor

@leandron Thanks for the review. Does this need a rebase?

I think it needs at least a force-push to re-trigger CI.

@masahi masahi merged commit aa2b37d into apache:main Sep 9, 2021
@mehrdadh mehrdadh deleted the microtvm-project-autotune branch September 9, 2021 14:46
ylc pushed a commit to ylc/tvm that referenced this pull request Sep 29, 2021
* Initial commit of API server impl.

* initial commit of api client

* Add TVM-side glue code to use Project API

* Change tvm.micro.Session to use Project API

* Rework how crt_config.h is used on the host.

 * use template crt_config.h for host test runtime; delete
   src/runtime/crt/host/crt_config.h so that it doesn't diverge from
   the template
 * bring template crt_config.h inline with the one actually in use
  * rename to MAX_STRLEN_DLTYPE
 * Create a dedicated TVM-side host crt_config.h in src/runtime/micro

* Modify Transport infrastructure to work with Project API

* Add host microTVM API server

* Zephyr implementation of microTVM API server

 * move all zephyr projects to apps/microtvm/zephyr/template_project

* consolidate CcompilerAnnotator

* Allow model library format with c backend, add test.

* Update unit tests

* fix incorrect doc

* Delete old Zephyr build infrastructure

* Delete old build abstractions

* Delete old Transport implementations and simplify module

* lint

* ASF header

* address gromero comments

* final fixes?

* fix is_shutdown

* fix user-facing API

* fix TempDirectory / operator

* Update micro_tflite tutorial

* lint

* fix test_crt and test_link_params

* undo global micro import, hopefully fix fixture

* lint

* fix more tests

* Add session_constructor_args to tracker request() function.

 * Allows tracker clients to open non-traditional RPC sessions

* Generate entry_func symbol in C host codegen.

 * Needed for AutoTVM.

* print MeasureErrorNo enum value in MeasureResult repr

* Add microTVM session constructor.

 * This constructor is to be called from the RPC driver to flash and
   connect to the RPC server on the microcontroller.

* add build_kwargs as a Builder constructor arg.

 * build_kwargs is derived from pre-configured args, the runner, and
   now from the script.
 * user-supplied build kwargs override the other two, and a warning is
   printed if any key is overridden.

* Add do_fork option to Builder, to support stateful builders

 * When AutoTVM builder forks, any global state modified by the
   build_func is lost between builds

* Checkin module_loader used to build and flash microTVM for autotuning.

* Import micro into top-level when enabled.

 * AutoTVM RPC server needs to load the micro session constructor.

* Add tvm.contrib.random.random_fill to microTVM.

 * Allows autotuning with random data.

* Move compilation to runner :O

* Add a tutorial for AutoTVM with microcontrollers.

* Fix si_prefix in autotuner callback

* black format and git-clang-format

* Switch tutorial back to qemu version

* improve error reporting so CI will show test error

* black format

* autotvm is working

* fix tutorial

* fix dependencies

* fix auto tune issue

* lint

* address comments

* fix lint

* test crt and zephyr added

* fix func registery size

* moved autotune test and fixed

* fix crt test

* address comments

* change relay text

* change relay in text_zephyr

* class added

* changed relay module in tutorial and cleanup

* address comments

* address TK comments

* change fork

* final comments

* retrigger due to flahy test

* fix tutorial

* retrigger

* fix changes due to merge

Co-authored-by: Andrew Reusch <areusch@octoml.ai>
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
* Initial commit of API server impl.

* initial commit of api client

* Add TVM-side glue code to use Project API

* Change tvm.micro.Session to use Project API

* Rework how crt_config.h is used on the host.

 * use template crt_config.h for host test runtime; delete
   src/runtime/crt/host/crt_config.h so that it doesn't diverge from
   the template
 * bring template crt_config.h inline with the one actually in use
  * rename to MAX_STRLEN_DLTYPE
 * Create a dedicated TVM-side host crt_config.h in src/runtime/micro

* Modify Transport infrastructure to work with Project API

* Add host microTVM API server

* Zephyr implementation of microTVM API server

 * move all zephyr projects to apps/microtvm/zephyr/template_project

* consolidate CcompilerAnnotator

* Allow model library format with c backend, add test.

* Update unit tests

* fix incorrect doc

* Delete old Zephyr build infrastructure

* Delete old build abstractions

* Delete old Transport implementations and simplify module

* lint

* ASF header

* address gromero comments

* final fixes?

* fix is_shutdown

* fix user-facing API

* fix TempDirectory / operator

* Update micro_tflite tutorial

* lint

* fix test_crt and test_link_params

* undo global micro import, hopefully fix fixture

* lint

* fix more tests

* Add session_constructor_args to tracker request() function.

 * Allows tracker clients to open non-traditional RPC sessions

* Generate entry_func symbol in C host codegen.

 * Needed for AutoTVM.

* print MeasureErrorNo enum value in MeasureResult repr

* Add microTVM session constructor.

 * This constructor is to be called from the RPC driver to flash and
   connect to the RPC server on the microcontroller.

* add build_kwargs as a Builder constructor arg.

 * build_kwargs is derived from pre-configured args, the runner, and
   now from the script.
 * user-supplied build kwargs override the other two, and a warning is
   printed if any key is overridden.

* Add do_fork option to Builder, to support stateful builders

 * When AutoTVM builder forks, any global state modified by the
   build_func is lost between builds

* Checkin module_loader used to build and flash microTVM for autotuning.

* Import micro into top-level when enabled.

 * AutoTVM RPC server needs to load the micro session constructor.

* Add tvm.contrib.random.random_fill to microTVM.

 * Allows autotuning with random data.

* Move compilation to runner :O

* Add a tutorial for AutoTVM with microcontrollers.

* Fix si_prefix in autotuner callback

* black format and git-clang-format

* Switch tutorial back to qemu version

* improve error reporting so CI will show test error

* black format

* autotvm is working

* fix tutorial

* fix dependencies

* fix auto tune issue

* lint

* address comments

* fix lint

* test crt and zephyr added

* fix func registery size

* moved autotune test and fixed

* fix crt test

* address comments

* change relay text

* change relay in text_zephyr

* class added

* changed relay module in tutorial and cleanup

* address comments

* address TK comments

* change fork

* final comments

* retrigger due to flahy test

* fix tutorial

* retrigger

* fix changes due to merge

Co-authored-by: Andrew Reusch <areusch@octoml.ai>
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.

None yet

5 participants