Skip to content

[AutoTVM] Use popenpool in local_executor#8851

Merged
vinx13 merged 9 commits into
apache:mainfrom
shingjan:autotvm_local_executor
Sep 8, 2021
Merged

[AutoTVM] Use popenpool in local_executor#8851
vinx13 merged 9 commits into
apache:mainfrom
shingjan:autotvm_local_executor

Conversation

@shingjan

@shingjan shingjan commented Aug 26, 2021

Copy link
Copy Markdown

This PR intends to replace multiprocessing.pool with popenpool in autotvm/local_executor.
@junrushao1994 @vinx13 @tqchen

@shingjan

shingjan commented Aug 26, 2021

Copy link
Copy Markdown
Author

Author's note:

  1. There is a weird ModuleNotFoundError: No module named 'test_autotvm_executor' during the execution of test_autotvm_executor.py. Still working on a fix.

  2. The reason why PopenPoolExecutor is used here instead of PopenWorker is bcoz using PopenWorker requires the re-implementaion of LocalFuture as well as a usable replacement of multiprocessing.Queue, which are all taken care of in PopenPoolExecutor.

@shingjan shingjan force-pushed the autotvm_local_executor branch from 5c0ace4 to bab59c0 Compare August 26, 2021 00:41
@vinx13

vinx13 commented Aug 26, 2021

Copy link
Copy Markdown
Member

Similar to the fix you did for auto scheduler, test_autotvm_executor is not really a module and it shouldn't be imported. Anything common for tests should probably be moved to tvm.testing.autotvm
Also, we may think again whether LocalExecutor and LocalFuture is really needed, if possible we prefer using PopenWorker or PopenPoolExecutor directly

@vinx13 vinx13 self-assigned this Aug 26, 2021
@shingjan shingjan requested a review from zhiics as a code owner August 26, 2021 00:56
@shingjan

Copy link
Copy Markdown
Author

@vinx13 just moved all testing utility functions of autotvm to tvm.testing. About LocalFuture and LocalExecutor, they do seem very much like PopenPoolExecutor now. Maybe we can sync offline about this.

@shingjan

Copy link
Copy Markdown
Author

@vinx13 The refactor is finished. I moved the two test cases for LocalExecutor to test_popen_pool.py as I do believe they can be of use, e.g. make sure the PopenPoolExecutor is functioning correctly. I also move the LocalFutureNoFork to autotvm.executor as one test case depends on it and it is for convenience's sake that we keep it.

@shingjan shingjan force-pushed the autotvm_local_executor branch from 18eab6b to 29f0c1b Compare August 26, 2021 23:23
@vinx13

vinx13 commented Aug 27, 2021

Copy link
Copy Markdown
Member

Can you confirm autotvm work both in the terminal and jupyter? You can run this tutorial

Comment thread python/tvm/autotvm/measure/measure_methods.py Outdated
Comment thread python/tvm/autotvm/measure/executor.py Outdated

@vinx13 vinx13 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The test case can be updated if the only usage of LocalFutureNoFork is the test case

Comment thread tests/python/unittest/test_autotvm_measure.py Outdated
Comment thread python/tvm/autotvm/measure/measure_methods.py Outdated
@shingjan shingjan force-pushed the autotvm_local_executor branch from c25bd00 to d78e79e Compare September 1, 2021 21:37
Comment thread python/tvm/autotvm/measure/measure_methods.py
@shingjan shingjan requested a review from vinx13 September 1, 2021 23:10
@shingjan

shingjan commented Sep 2, 2021

Copy link
Copy Markdown
Author

On 64-core linux machine, each did 5 runs:

tune_conv2d_cuda.py, n_trial = 192 This PR Main
Local builder + Local runner mean std mean std
Build time 10.438 1.935 8.21 3.165
Run time 27.452 3.349 26.248 6.114
Local builder + RPC runner
Build time 10.304 0.925 7.844 1.838
Run time 25.098 3.229 27.644 3.836

@vinx13

vinx13 commented Sep 2, 2021

Copy link
Copy Markdown
Member

@shingjan can you also check if custom build func still works for autotvm?

@vinx13 vinx13 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

found some changes needed

@shingjan shingjan force-pushed the autotvm_local_executor branch from 0b5397e to 34e9d12 Compare September 7, 2021 17:18
@shingjan shingjan requested a review from vinx13 September 7, 2021 17:20
@shingjan

shingjan commented Sep 7, 2021

Copy link
Copy Markdown
Author

With a custom build function like the one below, this PR runs fine for tune_conv2d_cuda:

from tvm.contrib import tar


def custom_build(*args):
    return tar.tar(*args)


custom_build.output_format = "tar"

measure_option = autotvm.measure_option(
    builder=autotvm.LocalBuilder(build_func=custom_build),
    runner=autotvm.LocalRunner(repeat=3, min_repeat_ms=100, timeout=4),
)

@vinx13 vinx13 merged commit 1f2fdbf into apache:main Sep 8, 2021
ylc pushed a commit to ylc/tvm that referenced this pull request Sep 29, 2021
* use popenpool in local_executor

* move auto_tvm_common to tvm.testing

* refactor

* nit

* remove LocalFutureNoFork

* exception handling

* handling two exceptions

* handling error

* add initiazlier
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
* use popenpool in local_executor

* move auto_tvm_common to tvm.testing

* refactor

* nit

* remove LocalFutureNoFork

* exception handling

* handling two exceptions

* handling error

* add initiazlier
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.

2 participants