You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/08/03 00:04:32 UTC
[GitHub] [tvm] shingjan opened a new pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
shingjan opened a new pull request #8492:
URL: https://github.com/apache/tvm/pull/8492
This PR intends to replace the use of `multiprocessing.pool.ThreadPool` to `popen_pool.PopenPoolExecutor` in `auto_scheduler`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tkonolige commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r673331852
##########
File path: python/tvm/autotvm/tuner/xgboost_cost_model.py
##########
@@ -161,7 +162,8 @@ def _reset_pool(self, space, target, task):
_extract_space = space
_extract_target = target
_extract_task = task
- self.pool = multiprocessing.Pool(self.num_threads)
+ self.pool = popen_pool.PopenPoolExecutor(self.num_threads)
+ # self.pool = multiprocessing.Pool(self.num_threads)
Review comment:
Remove this comment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tkonolige commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r682071759
##########
File path: python/tvm/auto_scheduler/utils.py
##########
@@ -289,41 +287,17 @@ def wrapper():
return res[0]
-def _func_wrapper(que, func, args, kwargs, add_thread_wrapper):
- """Call function and return the result over the queue."""
- try:
- if add_thread_wrapper:
- # Add a new layer of threadinng to avoid the conflict between
- # python's multiprocessing and tvm's thread pool.
- res = call_func_with_thread(func, args, kwargs)
- else:
- res = func(*args, **kwargs)
- que.put(res)
- except Exception: # pylint: disable=broad-except
- que.put(Exception(make_traceback_info()))
-
-
-def call_func_with_timeout(timeout, func, args=(), kwargs=None, add_thread_wrapper=False):
+def call_func_with_timeout(
+ timeout, func, args=(), kwargs=None, add_thread_wrapper=False
+): # pylint: disable=unused-argument
Review comment:
Seems like we should just remove `add_thread_wrapper`.
##########
File path: python/tvm/contrib/popen_pool.py
##########
@@ -269,9 +269,14 @@ class PopenPoolExecutor:
timeout : float
Timeout value for each function submit.
+ Note
+ ----
+ If max_workers is NONE then the number returned by
+ os.cpu_count() is used. This method aligns with the
+ behavior of multiprocessing.pool().
"""
- def __init__(self, max_workers, timeout=None):
+ def __init__(self, max_workers=os.cpu_count(), timeout=None):
Review comment:
TVM has its own function to get number of threads. Should we just use that code instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-897238970
Thanks for all your effort in this refactoring 🙏
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev commented on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
echuraev commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-911821916
I also met this problem. The problem is in new mechanism of creating processes and in serializing global object such as `BuildFunc`. I'm working on a fix for this problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-892170335
Thanks @shingjan! I just approve the github action running
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-911954249
Additionally, because some of the custom function might need to use functions from ```tvm.contrib.ndk```, and others are available(or we need to import them in the function body). The easiest way is to import them in popen_worker by default.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev edited a comment on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
echuraev edited a comment on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-911821916
@Wanger-SJTU I also met this problem. The problem is in new mechanism of creating processes and in serializing global object such as `BuildFunc`. I'm working on a fix for this problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r673338148
##########
File path: python/tvm/auto_scheduler/utils.py
##########
@@ -306,6 +307,7 @@ def _func_wrapper(que, func, args, kwargs, add_thread_wrapper):
def call_func_with_timeout(timeout, func, args=(), kwargs=None, add_thread_wrapper=False):
"""Call a function with timeout"""
que = multiprocessing.Queue(2)
+ process = PopenWorker()
Review comment:
yes, and `PopenWorker` supports for `timeout` argument in `send` which is a builtin mechanism of calling with timeout, so queue is no longer needed, this code block can be simplified
##########
File path: python/tvm/autotvm/tuner/xgboost_cost_model.py
##########
@@ -161,7 +162,8 @@ def _reset_pool(self, space, target, task):
_extract_space = space
_extract_target = target
_extract_task = task
- self.pool = multiprocessing.Pool(self.num_threads)
+ self.pool = popen_pool.PopenPoolExecutor(self.num_threads)
+ # self.pool = multiprocessing.Pool(self.num_threads)
Review comment:
remove this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 edited a comment on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 edited a comment on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-911879404
We should send the callable `build_func` here as an argument instead of the name of build_func. There will be a new requirement for custom build functions: it requires the custom build_func to be defined in a module importable from `PYTHONPATH`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-892814233
@shingjan Please fix the wildcard import issue, make sure it could run locally, and let's get this PR ready ASAP
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-882962453
By far those are all uses of `multiprocessing.pool` in this repo that I can find, is there any places that this change should also apply to? @junrushao1994 @vinx13 @tkonolige
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 edited a comment on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 edited a comment on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-892931482
Seeing these error messages:
```
Exception ignored in: <bound method XGBoostCostModel.__del__ of <tvm.autotvm.tuner.xgboost_cost_model.XGBoostCostModel object at 0x7f1731da9b00>>
Traceback (most recent call last):
File "/workspace/python/tvm/autotvm/tuner/xgboost_cost_model.py", line 353, in __del__
self._close_pool()
File "/workspace/python/tvm/autotvm/tuner/xgboost_cost_model.py", line 169, in _close_pool
self.pool.terminate()
AttributeError: 'PopenPoolExecutor' object has no attribute 'terminate'
```
We might want to make sure the unit tests could pass locally before going to the CI :-)
Examples of commands to reproduce the tests locally (it's annotated on each step of our Jenkins):
```
/PATH/TO/TVM/docker/bash.sh tlcpack/ci-cpu:v0.75 ./tests/scripts/task_config_build_cpu.sh
/PATH/TO/TVM/docker/bash.sh tlcpack/ci-cpu:v0.75 ./tests/scripts/task_cpp_unittest.sh
/PATH/TO/TVM/docker/bash.sh tlcpack/ci-cpu:v0.75 ./tests/scripts/task_build.sh build -j$(nproc)
/PATH/TO/TVM/docker/bash.sh tlcpack/ci-cpu:v0.75 ./tests/scripts/task_ci_setup.sh
/PATH/TO/TVM/docker/bash.sh tlcpack/ci-cpu:v0.75 ./tests/scripts/task_python_unittest.sh
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 edited a comment on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 edited a comment on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-911896677
Function is pickled by it's qualified name (that means the module of the function should be importable). It might be different in our case I'll double check
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r675238878
##########
File path: python/tvm/auto_scheduler/utils.py
##########
@@ -305,25 +306,15 @@ def _func_wrapper(que, func, args, kwargs, add_thread_wrapper):
def call_func_with_timeout(timeout, func, args=(), kwargs=None, add_thread_wrapper=False):
"""Call a function with timeout"""
- que = multiprocessing.Queue(2)
- process = multiprocessing.Process(
- target=_func_wrapper, args=(que, func, args, kwargs or {}, add_thread_wrapper)
- )
- process.start()
+ process = PopenWorker()
+ process.send(func, args, kwargs, timeout)
try:
- res = que.get(timeout=timeout)
- except queue.Empty:
- res = TimeoutError()
-
- # clean queue and process
- kill_child_processes(process.pid)
- process.terminate()
- process.join()
- que.close()
- que.join_thread()
- del process
- del que
+ res = process.recv()
+ except Exception as ex:
+ res = ex
Review comment:
I used a newly created exception containing the result returned by `make_traceback_info`. And this new exception will be returned as result.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r682089227
##########
File path: python/tvm/contrib/popen_pool.py
##########
@@ -269,9 +269,14 @@ class PopenPoolExecutor:
timeout : float
Timeout value for each function submit.
+ Note
+ ----
+ If max_workers is NONE then the number returned by
+ os.cpu_count() is used. This method aligns with the
+ behavior of multiprocessing.pool().
"""
- def __init__(self, max_workers, timeout=None):
+ def __init__(self, max_workers=os.cpu_count(), timeout=None):
Review comment:
I was thinking `multiprocessing.cpu_count()` but that is essentially a wrapper for `os.cpu_count()`. Would that be possible that you can point me to the place with tvm's own function to get number of threads?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-892931482
Seeing these error messages:
```
Exception ignored in: <bound method XGBoostCostModel.__del__ of <tvm.autotvm.tuner.xgboost_cost_model.XGBoostCostModel object at 0x7f1731da9b00>>
Traceback (most recent call last):
File "/workspace/python/tvm/autotvm/tuner/xgboost_cost_model.py", line 353, in __del__
self._close_pool()
File "/workspace/python/tvm/autotvm/tuner/xgboost_cost_model.py", line 169, in _close_pool
self.pool.terminate()
AttributeError: 'PopenPoolExecutor' object has no attribute 'terminate'
```
We might want to make sure the unit tests could pass locally before going to the CI :-)
Examples of commands to reproduce the tests locally (it's annotated on each step of our Jenkins):
```
docker/bash.sh tlcpack/ci-cpu:v0.75 ./tests/scripts/task_config_build_cpu.sh
docker/bash.sh tlcpack/ci-cpu:v0.75 ./tests/scripts/task_cpp_unittest.sh
docker/bash.sh tlcpack/ci-cpu:v0.75 ./tests/scripts/task_build.sh build -j$(nproc)
docker/bash.sh tlcpack/ci-cpu:v0.75 ./tests/scripts/task_ci_setup.sh
docker/bash.sh tlcpack/ci-cpu:v0.75 ./tests/scripts/task_python_unittest.sh
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tkonolige commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r687860950
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -664,15 +665,13 @@ def local_build_worker(args):
)
build_func = BuildFunc.build_func
- res = call_func_with_timeout(timeout, _timed_func, args=(inp, build_func, verbose))
Review comment:
Because you are no longer using `timeout` in this function, can you remove it from the arguments (and where it is passed in below -- line 710).
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -817,21 +818,66 @@ def prepare_input_map(args):
return tensor_input_map
+def prepare_runner_args(inp, build_res):
+ """This function prepares the pre-defined arguments in `TASK_INPUT_BUFFER_TABLE` for local/rpc
+ runner in main process
+
+ Parameters
+ ----------
+ inp : MeasureInput
+ Measure input to be measured.
+
+ build_res : BuildResult
+ Build result to be measured.
+
+ Returns
+ -------
+ List[Optional[numpy.ndarray]] :
+ List of arguments for running the program. If the argument does not have a pre-defined input
+ buffer, None is added to the list as a placeholder.
+
+ """
+ # pylint: disable=import-outside-toplevel
+ from .search_task import get_task_input_buffer # lazily import to avoid recursive dependency
+
+ task_input_names = inp.task.task_input_names
+ tensor_input_map = prepare_input_map(build_res.args)
+ if not task_input_names:
+ tensor_input_map = {}
+ args = []
+ task_inputs_count = 0
+ for arg in build_res.args:
+ if arg in tensor_input_map:
+ tensor_name = tensor_input_map[arg]
+ if tensor_name in task_input_names:
+ task_input_buffer = get_task_input_buffer(inp.task.workload_key, tensor_name)
+ # convert tvm.NDArray to picklable numpy.ndarray
+ args.append(task_input_buffer.numpy())
Review comment:
Was the previous approach pickling the input arrays? It seems like this could have a significant performance overhead. I think you can test this by tuning a sparse workload.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-882962453
By far those are all uses of `multiprocessing.pool` in this repo that I can find, is there any places that this change should also apply to? @junrushao1994 @vinx13 @tkonolige
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r685568508
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -62,6 +63,7 @@
deserialize_workload_registry_entry,
)
+
Review comment:
```suggestion
```
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -817,21 +819,63 @@ def prepare_input_map(args):
return tensor_input_map
+def prepare_runner_args(inp, build_res):
+ """This function prepare the arguments for local/rpc runner in main process
+ Parameters
+ ----------
+ inp : MeasureInput
+ Measure input to be measured.
+
+ build_res : BuildResult
+ Build result to be measured.
+
+ Returns
+ -------
+ List[NDArray, None] :
Review comment:
```suggestion
List[Optional[numpy.ndarray]] :
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r682174952
##########
File path: python/tvm/testing/__init__.py
##########
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=redefined-builtin, wildcard-import
+"""Utility Python functions for TVM testing"""
+from utils import *
Review comment:
I was just trying to get the subpackage to work on CI. Will avoid using wild card matching.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-892814233
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-890084788
`tvm.testing` sounds right
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan closed pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan closed pull request #8492:
URL: https://github.com/apache/tvm/pull/8492
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r683065520
##########
File path: python/tvm/testing/__init__.py
##########
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=redefined-builtin, wildcard-import
+"""Utility Python functions for TVM testing"""
+from utils import *
Review comment:
done
##########
File path: python/tvm/testing/__init__.py
##########
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=redefined-builtin, wildcard-import
+"""Utility Python functions for TVM testing"""
+from .utils import *
+from .auto_scheduler import *
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-895637298
For end-to-end correctness test, Wuwei and I ran this [tutorial](https://tvm.apache.org/docs/tutorials/auto_scheduler/tune_network_x86.html) with `tuning` set to both `True` and `False` to make sure these is no correctness issues.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r683754663
##########
File path: python/tvm/testing/__init__.py
##########
@@ -16,41 +16,17 @@
# under the License.
# pylint: disable=redefined-builtin, wildcard-import
"""Utility Python functions for TVM testing"""
-from .utils import (
- assert_allclose,
- assert_prim_expr_equal,
- check_bool_expr_is_true,
- check_int_constraints_trans_consistency,
- check_numerical_grads,
- device_enabled,
- device_test,
- echo,
- enabled_targets,
- exclude_targets,
- fixture,
- parameter,
- parameters,
- parametrize_targets,
- uses_gpu,
- known_failing_targets,
- object_use_count,
- requires_cuda,
- requires_cudagraph,
- requires_gpu,
- requires_llvm,
- requires_rocm,
- requires_rpc,
- requires_tensorcore,
- requires_metal,
- requires_micro,
- requires_opencl,
- test_check_eq_callback,
- test_raise_error_callback,
- test_wrap_callback,
- _auto_parametrize_target,
- _count_num_fixture_uses,
- _remove_global_fixture_definitions,
- _parametrize_correlated_parameters,
-)
+from .utils import assert_allclose, assert_prim_expr_equal, check_bool_expr_is_true
Review comment:
This is the import style I saw in other [__init__.py ](https://github.com/apache/tvm/blob/8302524a381655a727db74db97777cba99a2609e/python/tvm/te/__init__.py#L21). Should we keep this import style consistent with those? `FrontendTestModule` is added.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r682144316
##########
File path: python/tvm/testing/__init__.py
##########
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=redefined-builtin, wildcard-import
+"""Utility Python functions for TVM testing"""
+from .utils import *
+from .auto_scheduler import *
Review comment:
`__init__.py` is needed, and `testing.py` should be removed, otherwise it will shadow `tvm.testing` package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r681369839
##########
File path: tests/python/unittest/test_auto_scheduler_compute_dag.py
##########
@@ -23,7 +23,7 @@
from tvm import topi
from tvm import auto_scheduler, te
-from test_auto_scheduler_common import (
+from tvm.testing.auto_scheduler_common import (
Review comment:
let's name it `tvm.testing.auto_scheduler`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r672729248
##########
File path: python/tvm/auto_scheduler/utils.py
##########
@@ -306,6 +307,7 @@ def _func_wrapper(que, func, args, kwargs, add_thread_wrapper):
def call_func_with_timeout(timeout, func, args=(), kwargs=None, add_thread_wrapper=False):
"""Call a function with timeout"""
que = multiprocessing.Queue(2)
+ process = PopenWorker()
Review comment:
Should this multiprocessing also be changed to PopenPoolWorker? This change is similar to #7889
##########
File path: python/tvm/auto_scheduler/utils.py
##########
@@ -306,6 +307,7 @@ def _func_wrapper(que, func, args, kwargs, add_thread_wrapper):
def call_func_with_timeout(timeout, func, args=(), kwargs=None, add_thread_wrapper=False):
"""Call a function with timeout"""
que = multiprocessing.Queue(2)
+ process = PopenWorker()
Review comment:
Should this multiprocessing also be changed to PopenWorker? This change is similar to #7889
##########
File path: python/tvm/autotvm/record.py
##########
@@ -230,7 +231,7 @@ def split_workload(in_file, clean=True):
lines = list(open(in_file).readlines())
logger.info("start converting...")
- pool = multiprocessing.Pool()
+ pool = popen_pool.PopenPoolExecutor()
Review comment:
No number of threads/processes specified here.
##########
File path: python/tvm/autotvm/tuner/xgboost_cost_model.py
##########
@@ -161,7 +162,8 @@ def _reset_pool(self, space, target, task):
_extract_space = space
_extract_target = target
_extract_task = task
- self.pool = multiprocessing.Pool(self.num_threads)
+ self.pool = popen_pool.PopenPoolExecutor(self.num_threads)
Review comment:
`multiprocessing.pool()` seems to create a process pool instead of a thread pool. Hence the name of the argument passed here `self.num_threads` may not be the most accurate. Is that the case?
##########
File path: python/tvm/autotvm/utils.py
##########
@@ -111,7 +112,7 @@ def pool_map(func, args, batch_size, verbose=False, pool=None):
ret = None
tic = time.time()
- local_pool = pool or multiprocessing.Pool()
+ local_pool = pool or PopenPoolExecutor()
Review comment:
same as above. No specific number of processes here. We may consider adding a default number of process getting created in the implementation of `PopenPoolExecutor`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r672732382
##########
File path: python/tvm/autotvm/tuner/xgboost_cost_model.py
##########
@@ -161,7 +162,8 @@ def _reset_pool(self, space, target, task):
_extract_space = space
_extract_target = target
_extract_task = task
- self.pool = multiprocessing.Pool(self.num_threads)
+ self.pool = popen_pool.PopenPoolExecutor(self.num_threads)
Review comment:
yes it actually means num workers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r675212877
##########
File path: python/tvm/auto_scheduler/utils.py
##########
@@ -314,6 +314,8 @@ def call_func_with_timeout(timeout, func, args=(), kwargs=None, add_thread_wrapp
except Exception as ex:
res = ex
+ process.kill()
Review comment:
this is not needed as it is called in `POpenWorker.__del__`
##########
File path: python/tvm/auto_scheduler/utils.py
##########
@@ -306,6 +307,7 @@ def _func_wrapper(que, func, args, kwargs, add_thread_wrapper):
def call_func_with_timeout(timeout, func, args=(), kwargs=None, add_thread_wrapper=False):
"""Call a function with timeout"""
que = multiprocessing.Queue(2)
+ process = PopenWorker()
Review comment:
argument `add_thread_wrapper` can be removed, it uses `popen` to create a new process which won't cause conflict with tvm thread pool. cc @merrymercy @jcf94
##########
File path: python/tvm/auto_scheduler/utils.py
##########
@@ -305,25 +306,15 @@ def _func_wrapper(que, func, args, kwargs, add_thread_wrapper):
def call_func_with_timeout(timeout, func, args=(), kwargs=None, add_thread_wrapper=False):
"""Call a function with timeout"""
- que = multiprocessing.Queue(2)
- process = multiprocessing.Process(
- target=_func_wrapper, args=(que, func, args, kwargs or {}, add_thread_wrapper)
- )
- process.start()
+ process = PopenWorker()
+ process.send(func, args, kwargs, timeout)
try:
- res = que.get(timeout=timeout)
- except queue.Empty:
- res = TimeoutError()
-
- # clean queue and process
- kill_child_processes(process.pid)
- process.terminate()
- process.join()
- que.close()
- que.join_thread()
- del process
- del que
+ res = process.recv()
+ except Exception as ex:
+ res = ex
Review comment:
we'd still like to use `make_traceback_info` to make sure the exception has the same format
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Wanger-SJTU edited a comment on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
Wanger-SJTU edited a comment on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-911407652
@tqchen @shingjan @tkonolige @junrushao1994 @vinx13 this pr make ` builder=auto_scheduler.LocalBuilder(build_func="ndk" if use_ndk else "default")` fail. No matter what `build_func` specific, the `build_func` will fall back to `default`.
and with this pr, the demo code `tutorials/auto_scheduler/tune_network_arm.py` run failed with `use_ndk=true`
```
python tune_network_arm.py
default
> /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":
(Pdb) c
Get model...
Extract tasks...
Begin tuning...
ndk
> /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":
(Pdb) c
Get devices for measurement successfully!
> /home/framework/tvm/tutorials/auto_scheduler/tune_network_arm.py(326)tune_and_evaluate()
-> tuner.tune(tune_option)
(Pdb) c
----------------------------------------------------------------------
------------------------------ [ Task Scheduler ]
----------------------------------------------------------------------
| ID | Latency (ms) | Speed (GFLOPS) | Trials |
-------------------------------------------------
| 0 | - | - | 0 |
| 1 | - | - | 0 |
| 2 | - | - | 0 |
| 3 | - | - | 0 |
| 4 | - | - | 0 |
| 5 | - | - | 0 |
| 6 | - | - | 0 |
| 7 | - | - | 0 |
| 8 | - | - | 0 |
| 9 | - | - | 0 |
| 10 | - | - | 0 |
| 11 | - | - | 0 |
| 12 | - | - | 0 |
| 13 | - | - | 0 |
| 14 | - | - | 0 |
| 15 | - | - | 0 |
| 16 | - | - | 0 |
| 17 | - | - | 0 |
| 18 | - | - | 0 |
| 19 | - | - | 0 |
| 20 | - | - | 0 |
| 21 | - | - | 0 |
-------------------------------------------------
Estimated total latency: - ms Trials: 0 Used time : 1 s Next ID: 0
----------------------------------------------------------------------
------------------------------ [ Search ]
----------------------------------------------------------------------
Generate Sketches #s: 3
Sample Initial Population #s: 1658 fail_ct: 194 Time elapsed: 0.97
GA Iter: 0 Max score: 0.9999 Min score: 0.9896 #Pop: 18 #M+: 0 #M-: 0
GA Iter: 4 Max score: 1.0000 Min score: 0.9985 #Pop: 18 #M+: 1374 #M-: 76
EvolutionarySearch #s: 18 Time elapsed: 3.82
----------------------------------------------------------------------
------------------------------ [ Measure ]
----------------------------------------------------------------------
Get 9 programs to measure:
default
default
> /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":default
> /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":(Pdb)
(Pdb) > /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":
(Pdb) default
> /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":
(Pdb) default
> /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":
(Pdb) default
> /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":
(Pdb) default
default
> /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":
(Pdb) > /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":
(Pdb)
Traceback (most recent call last):
File "/home/anaconda3/lib/python3.8/runpy.py", line 194, in _run_module_as_main
return _run_code(code, main_globals, None,
File "/home/anaconda3/lib/python3.8/runpy.py", line 87, in _run_code
exec(code, run_globals)
File "/home/framework/tvm/python/tvm/exec/popen_worker.py", line 105, in <module>
main()
File "/home/framework/tvm/python/tvm/exec/popen_worker.py", line 77, in main
fn, args, kwargs, timeout = cloudpickle.loads(reader.read(bytes_size))
File "/home/framework/tvm/python/tvm/auto_scheduler/__init__.py", line 21, in <module>
from . import dispatcher
File "/home/framework/tvm/python/tvm/auto_scheduler/dispatcher.py", line 37, in <module>
from .search_task import SearchTask, TuningOptions
File "/home/framework/tvm/python/tvm/auto_scheduler/search_task.py", line 635, in <module>
def auto_schedule(task, search_policy=None, tuning_options=TuningOptions()):
File "/home/framework/tvm/python/tvm/auto_scheduler/search_task.py", line 184, in __init__
builder = LocalBuilder()
File "/home/framework/tvm/python/tvm/auto_scheduler/measure.py", line 341, in __init__
if build_func == "default":
File "/home/framework/tvm/python/tvm/auto_scheduler/measure.py", line 341, in __init__
if build_func == "default":
File "/home/anaconda3/lib/python3.8/bdb.py", line 88, in trace_dispatch
return self.dispatch_line(frame)
File "/home/anaconda3/lib/python3.8/bdb.py", line 113, in dispatch_line
if self.quitting: raise BdbQuit
bdb.BdbQuit
default
> /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":
(Pdb)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r681996545
##########
File path: tests/python/unittest/test_auto_scheduler_compute_dag.py
##########
@@ -23,7 +23,7 @@
from tvm import topi
from tvm import auto_scheduler, te
-from test_auto_scheduler_common import (
+from tvm.testing.auto_scheduler_common import (
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev commented on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
echuraev commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-911867044
> @shingjan @vinx13 would you guys take a closer look at the ndk issue as well?
This problem is not only related to the ndk. I suppose that any custom-built functions won't work on the main branch now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-911896677
Function is pickled by it's qualified name (that means the module of the function should be importable)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r682115085
##########
File path: python/tvm/contrib/popen_pool.py
##########
@@ -269,9 +269,14 @@ class PopenPoolExecutor:
timeout : float
Timeout value for each function submit.
+ Note
+ ----
+ If max_workers is NONE then the number returned by
+ os.cpu_count() is used. This method aligns with the
+ behavior of multiprocessing.pool().
"""
- def __init__(self, max_workers, timeout=None):
+ def __init__(self, max_workers=os.cpu_count(), timeout=None):
Review comment:
Let's make sure the behavior is consistent in this PR.
Just a nitpick, I would prefer defaulting `max_worker=None`, and do the checks in the function body, so that the signature could be a tiny little bit easier to read:
```python
if max_workers is None:
max_workers = os.cpu_count()
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r672730216
##########
File path: python/tvm/autotvm/utils.py
##########
@@ -111,7 +112,7 @@ def pool_map(func, args, batch_size, verbose=False, pool=None):
ret = None
tic = time.time()
- local_pool = pool or multiprocessing.Pool()
+ local_pool = pool or PopenPoolExecutor()
Review comment:
same as above. No specific number of processes here. We may consider adding a default number of process getting created in the implementation of `PopenPoolExecutor`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r675214603
##########
File path: python/tvm/auto_scheduler/utils.py
##########
@@ -306,6 +307,7 @@ def _func_wrapper(que, func, args, kwargs, add_thread_wrapper):
def call_func_with_timeout(timeout, func, args=(), kwargs=None, add_thread_wrapper=False):
"""Call a function with timeout"""
que = multiprocessing.Queue(2)
+ process = PopenWorker()
Review comment:
argument `add_thread_wrapper` can be removed (also `func_wrapper`), it uses `popen` to create a new process which won't cause conflict with tvm thread pool. cc @merrymercy @jcf94
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-890078347
Exception ignored in: <function XGBoostCostModel.__del__ at 0x7f93082e7b00>
Traceback (most recent call last):
File "/Users/yjshi/Repo/tvm/python/tvm/autotvm/tuner/xgboost_cost_model.py", line 353, in __del__
File "/Users/yjshi/Repo/tvm/python/tvm/autotvm/tuner/xgboost_cost_model.py", line 168, in _close_pool
AttributeError: 'XGBoostCostModel' object has no attribute 'pool'
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r672729462
##########
File path: python/tvm/autotvm/record.py
##########
@@ -230,7 +231,7 @@ def split_workload(in_file, clean=True):
lines = list(open(in_file).readlines())
logger.info("start converting...")
- pool = multiprocessing.Pool()
+ pool = popen_pool.PopenPoolExecutor()
Review comment:
No number of threads/processes specified here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r682121988
##########
File path: python/tvm/testing/__init__.py
##########
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=redefined-builtin, wildcard-import
+"""Utility Python functions for TVM testing"""
+from .utils import *
+from .auto_scheduler import *
Review comment:
As using `__init__.py` in `testing` will confuse the references to `tvm.testing`, I am removing it from this pr and opening up a new pr that contains this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tkonolige commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
tkonolige commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-881757161
What is the purpose of switching from ThreadPool to PopenPoolExecutor?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r682113911
##########
File path: python/tvm/testing/__init__.py
##########
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=redefined-builtin, wildcard-import
+"""Utility Python functions for TVM testing"""
+from .utils import *
+from .auto_scheduler import *
Review comment:
let's use `from . import auto_scheduler` instead
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r685604078
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -817,21 +818,63 @@ def prepare_input_map(args):
return tensor_input_map
+def prepare_runner_args(inp, build_res):
+ """This function prepare the arguments for local/rpc runner in main process
Review comment:
done
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -817,21 +818,63 @@ def prepare_input_map(args):
return tensor_input_map
+def prepare_runner_args(inp, build_res):
+ """This function prepare the arguments for local/rpc runner in main process
+ Parameters
+ ----------
+ inp : MeasureInput
+ Measure input to be measured.
+
+ build_res : BuildResult
+ Build result to be measured.
+
+ Returns
+ -------
+ List[Optional[numpy.ndarray]] :
+ List of numpy.ndarray representation of task input buffer. None if argument not present in task_input_names.
Review comment:
done
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -817,21 +819,63 @@ def prepare_input_map(args):
return tensor_input_map
+def prepare_runner_args(inp, build_res):
+ """This function prepare the arguments for local/rpc runner in main process
+ Parameters
+ ----------
+ inp : MeasureInput
+ Measure input to be measured.
+
+ build_res : BuildResult
+ Build result to be measured.
+
+ Returns
+ -------
+ List[NDArray, None] :
Review comment:
done
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -62,6 +63,7 @@
deserialize_workload_registry_entry,
)
+
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-885319745
please fix ci error
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-897234702
would be great if @comaniac @jcf94 @merrymercy could also take a look
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r687983298
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -664,15 +665,13 @@ def local_build_worker(args):
)
build_func = BuildFunc.build_func
- res = call_func_with_timeout(timeout, _timed_func, args=(inp, build_func, verbose))
Review comment:
`timeout` is still used in line 674 for building the result if an exception is thrown. Would that be okay if we keep this argument here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-882962453
By far those are all uses of `multiprocessing.pool` in this repo that I can find, is there any places that this change should also apply to? @junrushao1994 @vinx13 @tkonolige
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-892819421
@vinx13 Would you like to take some extra caution, try if local/rpc runner of both AutoTVM and Ansor could run smoothly before approving the PR? Thanks a lot 🙏
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r682164473
##########
File path: python/tvm/testing/__init__.py
##########
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=redefined-builtin, wildcard-import
+"""Utility Python functions for TVM testing"""
+from utils import *
Review comment:
Should use
```suggestion
from utils import *
```
BTW, do we really want wildcard import?
##########
File path: python/tvm/testing/__init__.py
##########
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=redefined-builtin, wildcard-import
+"""Utility Python functions for TVM testing"""
+from utils import *
Review comment:
Should use
```suggestion
from .utils import *
```
BTW, do we really want wildcard import?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r682110357
##########
File path: python/tvm/testing/utils.py
##########
@@ -0,0 +1,1290 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=invalid-name,unnecessary-comprehension
+""" TVM testing utilities
Review comment:
yes the `python/tvm/testing.py` is renamed and moved here. But I kept the old `testing.py` and didn't change the references to it as I think that piece of work should belong to a new pr.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev commented on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
echuraev commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-911888558
> We should send the callable `build_func` here as an argument instead of the name of build_func. There will be a new requirement for custom build functions: it requires the custom build_func to be defined in a module importable from `PYTHONPATH`
Are you sure that cloud pickle will be able to serialize such callable object? It was my first idea, but I faced the problem that I wasn't able to serialize python callable object and then call it from C++ code. I got an error like something like that: "Not able to serialize callable object". Don't remember the whole test, unfortunately. But maybe I missed something and did something wrong.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r682123519
##########
File path: python/tvm/contrib/popen_pool.py
##########
@@ -269,9 +269,14 @@ class PopenPoolExecutor:
timeout : float
Timeout value for each function submit.
+ Note
+ ----
+ If max_workers is NONE then the number returned by
+ os.cpu_count() is used. This method aligns with the
+ behavior of multiprocessing.pool().
"""
- def __init__(self, max_workers, timeout=None):
+ def __init__(self, max_workers=os.cpu_count(), timeout=None):
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r685596087
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -817,21 +818,63 @@ def prepare_input_map(args):
return tensor_input_map
+def prepare_runner_args(inp, build_res):
+ """This function prepare the arguments for local/rpc runner in main process
+ Parameters
+ ----------
+ inp : MeasureInput
+ Measure input to be measured.
+
+ build_res : BuildResult
+ Build result to be measured.
+
+ Returns
+ -------
+ List[Optional[numpy.ndarray]] :
+ List of numpy.ndarray representation of task input buffer. None if argument not present in task_input_names.
Review comment:
```suggestion
List of arguments for running the program. If the argument does not have a pre-defined input
buffer, None is added to the list as a placeholder.
```
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -817,21 +818,63 @@ def prepare_input_map(args):
return tensor_input_map
+def prepare_runner_args(inp, build_res):
+ """This function prepare the arguments for local/rpc runner in main process
Review comment:
```suggestion
"""This function prepares the pre-defined arguments in `TASK_INPUT_BUFFER_TABLE` for local/rpc
runner in main process.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Wanger-SJTU commented on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
Wanger-SJTU commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-911407652
@tqchen @shingjan @tkonolige @junrushao1994 @vinx13 this pr make ` builder=auto_scheduler.LocalBuilder(build_func="ndk" if use_ndk else "default")` fail. No matter what `build_func` specific, the `build_func` will fall back to `default`.
and with this pr, the demo code `tutorials/auto_scheduler/tune_network_arm.py` run failed.
```
python tune_network_arm.py
default
> /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":
(Pdb) c
Get model...
Extract tasks...
Begin tuning...
ndk
> /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":
(Pdb) c
Get devices for measurement successfully!
> /home/framework/tvm/tutorials/auto_scheduler/tune_network_arm.py(326)tune_and_evaluate()
-> tuner.tune(tune_option)
(Pdb) c
----------------------------------------------------------------------
------------------------------ [ Task Scheduler ]
----------------------------------------------------------------------
| ID | Latency (ms) | Speed (GFLOPS) | Trials |
-------------------------------------------------
| 0 | - | - | 0 |
| 1 | - | - | 0 |
| 2 | - | - | 0 |
| 3 | - | - | 0 |
| 4 | - | - | 0 |
| 5 | - | - | 0 |
| 6 | - | - | 0 |
| 7 | - | - | 0 |
| 8 | - | - | 0 |
| 9 | - | - | 0 |
| 10 | - | - | 0 |
| 11 | - | - | 0 |
| 12 | - | - | 0 |
| 13 | - | - | 0 |
| 14 | - | - | 0 |
| 15 | - | - | 0 |
| 16 | - | - | 0 |
| 17 | - | - | 0 |
| 18 | - | - | 0 |
| 19 | - | - | 0 |
| 20 | - | - | 0 |
| 21 | - | - | 0 |
-------------------------------------------------
Estimated total latency: - ms Trials: 0 Used time : 1 s Next ID: 0
----------------------------------------------------------------------
------------------------------ [ Search ]
----------------------------------------------------------------------
Generate Sketches #s: 3
Sample Initial Population #s: 1658 fail_ct: 194 Time elapsed: 0.97
GA Iter: 0 Max score: 0.9999 Min score: 0.9896 #Pop: 18 #M+: 0 #M-: 0
GA Iter: 4 Max score: 1.0000 Min score: 0.9985 #Pop: 18 #M+: 1374 #M-: 76
EvolutionarySearch #s: 18 Time elapsed: 3.82
----------------------------------------------------------------------
------------------------------ [ Measure ]
----------------------------------------------------------------------
Get 9 programs to measure:
default
default
> /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":default
> /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":(Pdb)
(Pdb) > /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":
(Pdb) default
> /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":
(Pdb) default
> /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":
(Pdb) default
> /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":
(Pdb) default
default
> /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":
(Pdb) > /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":
(Pdb)
Traceback (most recent call last):
File "/home/anaconda3/lib/python3.8/runpy.py", line 194, in _run_module_as_main
return _run_code(code, main_globals, None,
File "/home/anaconda3/lib/python3.8/runpy.py", line 87, in _run_code
exec(code, run_globals)
File "/home/framework/tvm/python/tvm/exec/popen_worker.py", line 105, in <module>
main()
File "/home/framework/tvm/python/tvm/exec/popen_worker.py", line 77, in main
fn, args, kwargs, timeout = cloudpickle.loads(reader.read(bytes_size))
File "/home/framework/tvm/python/tvm/auto_scheduler/__init__.py", line 21, in <module>
from . import dispatcher
File "/home/framework/tvm/python/tvm/auto_scheduler/dispatcher.py", line 37, in <module>
from .search_task import SearchTask, TuningOptions
File "/home/framework/tvm/python/tvm/auto_scheduler/search_task.py", line 635, in <module>
def auto_schedule(task, search_policy=None, tuning_options=TuningOptions()):
File "/home/framework/tvm/python/tvm/auto_scheduler/search_task.py", line 184, in __init__
builder = LocalBuilder()
File "/home/framework/tvm/python/tvm/auto_scheduler/measure.py", line 341, in __init__
if build_func == "default":
File "/home/framework/tvm/python/tvm/auto_scheduler/measure.py", line 341, in __init__
if build_func == "default":
File "/home/anaconda3/lib/python3.8/bdb.py", line 88, in trace_dispatch
return self.dispatch_line(frame)
File "/home/anaconda3/lib/python3.8/bdb.py", line 113, in dispatch_line
if self.quitting: raise BdbQuit
bdb.BdbQuit
default
> /home/framework/tvm/python/tvm/auto_scheduler/measure.py(341)__init__()
-> if build_func == "default":
(Pdb)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 edited a comment on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 edited a comment on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-892819421
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r675215521
##########
File path: python/tvm/auto_scheduler/utils.py
##########
@@ -306,6 +307,7 @@ def _func_wrapper(que, func, args, kwargs, add_thread_wrapper):
def call_func_with_timeout(timeout, func, args=(), kwargs=None, add_thread_wrapper=False):
"""Call a function with timeout"""
que = multiprocessing.Queue(2)
+ process = PopenWorker()
Review comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r682112139
##########
File path: python/tvm/testing/__init__.py
##########
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=redefined-builtin, wildcard-import
+"""Utility Python functions for TVM testing"""
+from .utils import *
+from .auto_scheduler import *
Review comment:
I am not in favor of wildcard import here, although it is in the testing folder :-) Let's import only when necessary here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r672729248
##########
File path: python/tvm/auto_scheduler/utils.py
##########
@@ -306,6 +307,7 @@ def _func_wrapper(que, func, args, kwargs, add_thread_wrapper):
def call_func_with_timeout(timeout, func, args=(), kwargs=None, add_thread_wrapper=False):
"""Call a function with timeout"""
que = multiprocessing.Queue(2)
+ process = PopenWorker()
Review comment:
Should this multiprocessing also be changed to PopenPoolWorker? This change is similar to #7889
##########
File path: python/tvm/auto_scheduler/utils.py
##########
@@ -306,6 +307,7 @@ def _func_wrapper(que, func, args, kwargs, add_thread_wrapper):
def call_func_with_timeout(timeout, func, args=(), kwargs=None, add_thread_wrapper=False):
"""Call a function with timeout"""
que = multiprocessing.Queue(2)
+ process = PopenWorker()
Review comment:
Should this multiprocessing also be changed to PopenWorker? This change is similar to #7889
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-890082401
> auto_scheduler need to be fixed:
> File "/workspace/python/tvm/exec/popen_worker.py", line 77, in main
> fn, args, kwargs, timeout = cloudpickle.loads(reader.read(bytes_size))
> ModuleNotFoundError: No module named 'test_auto_scheduler_common'
Discussed with @shingjan , we should avoid importing non-package file in the test directory when writing tests, these common code should be put in `tvm.testing` or `tvm.auto_scheduler.testing`. Otherwise subprocesses created through popenpool will not be able to import them as they are not in `PYTHONPATH` cc @tqchen @junrushao1994
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-897074773
Thanks for the numbers! So I assume there is no regression :-) Please fix the lint and let’s get it finally merged
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-911879404
We should sent the callable `build_func` here as an argument instead of the name of build_func. There will be a new requirement for custom build functions: it requires the custom build_func to be defined in a module importable from `PYTHONPATH`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-911949552
we can serialize callable functions(through cloud pickle), but the requirement is that the callable functions can only call functions in the tvm namespace and should be available in https://github.com/apache/tvm/blob/main/python/tvm/exec/popen_worker.py
It does mean that if we call functions that are not by default imported, we might need to do
```c++
def custom_func():
# import here so things are available in the callee side
import dep1
dep1.xyz
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r683754663
##########
File path: python/tvm/testing/__init__.py
##########
@@ -16,41 +16,17 @@
# under the License.
# pylint: disable=redefined-builtin, wildcard-import
"""Utility Python functions for TVM testing"""
-from .utils import (
- assert_allclose,
- assert_prim_expr_equal,
- check_bool_expr_is_true,
- check_int_constraints_trans_consistency,
- check_numerical_grads,
- device_enabled,
- device_test,
- echo,
- enabled_targets,
- exclude_targets,
- fixture,
- parameter,
- parameters,
- parametrize_targets,
- uses_gpu,
- known_failing_targets,
- object_use_count,
- requires_cuda,
- requires_cudagraph,
- requires_gpu,
- requires_llvm,
- requires_rocm,
- requires_rpc,
- requires_tensorcore,
- requires_metal,
- requires_micro,
- requires_opencl,
- test_check_eq_callback,
- test_raise_error_callback,
- test_wrap_callback,
- _auto_parametrize_target,
- _count_num_fixture_uses,
- _remove_global_fixture_definitions,
- _parametrize_correlated_parameters,
-)
+from .utils import assert_allclose, assert_prim_expr_equal, check_bool_expr_is_true
Review comment:
This is the import style I saw in other [__init__.py ](https://github.com/apache/tvm/blob/8302524a381655a727db74db97777cba99a2609e/python/tvm/te/__init__.py#L21). FrontendTestModule is added.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tkonolige commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r688035416
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -664,15 +665,13 @@ def local_build_worker(args):
)
build_func = BuildFunc.build_func
- res = call_func_with_timeout(timeout, _timed_func, args=(inp, build_func, verbose))
Review comment:
Oh that makes sense. I missed that usage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r682108339
##########
File path: python/tvm/testing/utils.py
##########
@@ -0,0 +1,1290 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=invalid-name,unnecessary-comprehension
+""" TVM testing utilities
Review comment:
BTW, are the contents moved from anywhere else?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-881775375
PopenPool is intended to replace multiprocessing (not multithreading), so just curious, do we have specific reasons here to replace the ThreadPool here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r682091115
##########
File path: python/tvm/auto_scheduler/utils.py
##########
@@ -289,41 +287,17 @@ def wrapper():
return res[0]
-def _func_wrapper(que, func, args, kwargs, add_thread_wrapper):
- """Call function and return the result over the queue."""
- try:
- if add_thread_wrapper:
- # Add a new layer of threadinng to avoid the conflict between
- # python's multiprocessing and tvm's thread pool.
- res = call_func_with_thread(func, args, kwargs)
- else:
- res = func(*args, **kwargs)
- que.put(res)
- except Exception: # pylint: disable=broad-except
- que.put(Exception(make_traceback_info()))
-
-
-def call_func_with_timeout(timeout, func, args=(), kwargs=None, add_thread_wrapper=False):
+def call_func_with_timeout(
+ timeout, func, args=(), kwargs=None, add_thread_wrapper=False
+): # pylint: disable=unused-argument
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tkonolige commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r687938717
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -817,21 +818,66 @@ def prepare_input_map(args):
return tensor_input_map
+def prepare_runner_args(inp, build_res):
+ """This function prepares the pre-defined arguments in `TASK_INPUT_BUFFER_TABLE` for local/rpc
+ runner in main process
+
+ Parameters
+ ----------
+ inp : MeasureInput
+ Measure input to be measured.
+
+ build_res : BuildResult
+ Build result to be measured.
+
+ Returns
+ -------
+ List[Optional[numpy.ndarray]] :
+ List of arguments for running the program. If the argument does not have a pre-defined input
+ buffer, None is added to the list as a placeholder.
+
+ """
+ # pylint: disable=import-outside-toplevel
+ from .search_task import get_task_input_buffer # lazily import to avoid recursive dependency
+
+ task_input_names = inp.task.task_input_names
+ tensor_input_map = prepare_input_map(build_res.args)
+ if not task_input_names:
+ tensor_input_map = {}
+ args = []
+ task_inputs_count = 0
+ for arg in build_res.args:
+ if arg in tensor_input_map:
+ tensor_name = tensor_input_map[arg]
+ if tensor_name in task_input_names:
+ task_input_buffer = get_task_input_buffer(inp.task.workload_key, tensor_name)
+ # convert tvm.NDArray to picklable numpy.ndarray
+ args.append(task_input_buffer.numpy())
Review comment:
Oh, I missed that you tested with sparse. My bad!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-897066656
Benchmark
running `tune_sparse_x86.py` from tutorial on 64-core Linux machine
| This PR | | Main |
-- | -- | -- | -- | --
| Mean (sec) | Std | Mean (sec) | Std
Local Builder + Local Runner | | |
Build | 6.63 | 0.79 | 6.52 | 0.85
Run | 23.11 | 0.58 | 25.26 | 1.21
| | | |
Local Builder + RPC Runner | | |
Build | 6.72 | 0.9 | 5.55 | 0.54
Run | 38.29 | 0.94 | 38.0 | 0.99
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r673350114
##########
File path: python/tvm/autotvm/tuner/xgboost_cost_model.py
##########
@@ -161,7 +162,8 @@ def _reset_pool(self, space, target, task):
_extract_space = space
_extract_target = target
_extract_task = task
- self.pool = multiprocessing.Pool(self.num_threads)
+ self.pool = popen_pool.PopenPoolExecutor(self.num_threads)
+ # self.pool = multiprocessing.Pool(self.num_threads)
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r683724229
##########
File path: python/tvm/testing/__init__.py
##########
@@ -16,41 +16,17 @@
# under the License.
# pylint: disable=redefined-builtin, wildcard-import
"""Utility Python functions for TVM testing"""
-from .utils import (
- assert_allclose,
- assert_prim_expr_equal,
- check_bool_expr_is_true,
- check_int_constraints_trans_consistency,
- check_numerical_grads,
- device_enabled,
- device_test,
- echo,
- enabled_targets,
- exclude_targets,
- fixture,
- parameter,
- parameters,
- parametrize_targets,
- uses_gpu,
- known_failing_targets,
- object_use_count,
- requires_cuda,
- requires_cudagraph,
- requires_gpu,
- requires_llvm,
- requires_rocm,
- requires_rpc,
- requires_tensorcore,
- requires_metal,
- requires_micro,
- requires_opencl,
- test_check_eq_callback,
- test_raise_error_callback,
- test_wrap_callback,
- _auto_parametrize_target,
- _count_num_fixture_uses,
- _remove_global_fixture_definitions,
- _parametrize_correlated_parameters,
-)
+from .utils import assert_allclose, assert_prim_expr_equal, check_bool_expr_is_true
Review comment:
let's keep the original style (merge everything in a single import)
```
from .utils import (
...
...
)
```
we also need to import everything registered via `TVM_REGISTER_GLOBAL` here https://github.com/apache/tvm/blob/main/src/support/ffi_testing.cc#L152 e.g.
```
from ._ffi_api import (
nop, echo,
...
)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-890076155
auto_scheduler need to be fixed:
File "/workspace/python/tvm/exec/popen_worker.py", line 77, in main
fn, args, kwargs, timeout = cloudpickle.loads(reader.read(bytes_size))
ModuleNotFoundError: No module named 'test_auto_scheduler_common'
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r683754663
##########
File path: python/tvm/testing/__init__.py
##########
@@ -16,41 +16,17 @@
# under the License.
# pylint: disable=redefined-builtin, wildcard-import
"""Utility Python functions for TVM testing"""
-from .utils import (
- assert_allclose,
- assert_prim_expr_equal,
- check_bool_expr_is_true,
- check_int_constraints_trans_consistency,
- check_numerical_grads,
- device_enabled,
- device_test,
- echo,
- enabled_targets,
- exclude_targets,
- fixture,
- parameter,
- parameters,
- parametrize_targets,
- uses_gpu,
- known_failing_targets,
- object_use_count,
- requires_cuda,
- requires_cudagraph,
- requires_gpu,
- requires_llvm,
- requires_rocm,
- requires_rpc,
- requires_tensorcore,
- requires_metal,
- requires_micro,
- requires_opencl,
- test_check_eq_callback,
- test_raise_error_callback,
- test_wrap_callback,
- _auto_parametrize_target,
- _count_num_fixture_uses,
- _remove_global_fixture_definitions,
- _parametrize_correlated_parameters,
-)
+from .utils import assert_allclose, assert_prim_expr_equal, check_bool_expr_is_true
Review comment:
This is the import style I saw in other [__init__.py ](https://github.com/apache/tvm/blob/8302524a381655a727db74db97777cba99a2609e/python/tvm/te/__init__.py#L21). Should we keep this import style consistent with those? FrontendTestModule is added.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r683724229
##########
File path: python/tvm/testing/__init__.py
##########
@@ -16,41 +16,17 @@
# under the License.
# pylint: disable=redefined-builtin, wildcard-import
"""Utility Python functions for TVM testing"""
-from .utils import (
- assert_allclose,
- assert_prim_expr_equal,
- check_bool_expr_is_true,
- check_int_constraints_trans_consistency,
- check_numerical_grads,
- device_enabled,
- device_test,
- echo,
- enabled_targets,
- exclude_targets,
- fixture,
- parameter,
- parameters,
- parametrize_targets,
- uses_gpu,
- known_failing_targets,
- object_use_count,
- requires_cuda,
- requires_cudagraph,
- requires_gpu,
- requires_llvm,
- requires_rocm,
- requires_rpc,
- requires_tensorcore,
- requires_metal,
- requires_micro,
- requires_opencl,
- test_check_eq_callback,
- test_raise_error_callback,
- test_wrap_callback,
- _auto_parametrize_target,
- _count_num_fixture_uses,
- _remove_global_fixture_definitions,
- _parametrize_correlated_parameters,
-)
+from .utils import assert_allclose, assert_prim_expr_equal, check_bool_expr_is_true
Review comment:
let's keep the original style (merge everything in a single import)
```
from .utils import (
...
...
)
```
we also need to import everything registered via `TVM_REGISTER_GLOBAL` here https://github.com/apache/tvm/blob/main/src/support/ffi_testing.cc#L152 e.g.
```
from ._ffi_utils import (
nop, echo,
...
)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-896296277
Sounds good. Let’s get some numbers with a large multi-core instance
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r672732382
##########
File path: python/tvm/autotvm/tuner/xgboost_cost_model.py
##########
@@ -161,7 +162,8 @@ def _reset_pool(self, space, target, task):
_extract_space = space
_extract_target = target
_extract_task = task
- self.pool = multiprocessing.Pool(self.num_threads)
+ self.pool = popen_pool.PopenPoolExecutor(self.num_threads)
Review comment:
yes it actually means num workers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r672732382
##########
File path: python/tvm/autotvm/tuner/xgboost_cost_model.py
##########
@@ -161,7 +162,8 @@ def _reset_pool(self, space, target, task):
_extract_space = space
_extract_target = target
_extract_task = task
- self.pool = multiprocessing.Pool(self.num_threads)
+ self.pool = popen_pool.PopenPoolExecutor(self.num_threads)
Review comment:
yes it actually means num workers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r683065520
##########
File path: python/tvm/testing/__init__.py
##########
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=redefined-builtin, wildcard-import
+"""Utility Python functions for TVM testing"""
+from utils import *
Review comment:
done
##########
File path: python/tvm/testing/__init__.py
##########
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=redefined-builtin, wildcard-import
+"""Utility Python functions for TVM testing"""
+from .utils import *
+from .auto_scheduler import *
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r672729248
##########
File path: python/tvm/auto_scheduler/utils.py
##########
@@ -306,6 +307,7 @@ def _func_wrapper(que, func, args, kwargs, add_thread_wrapper):
def call_func_with_timeout(timeout, func, args=(), kwargs=None, add_thread_wrapper=False):
"""Call a function with timeout"""
que = multiprocessing.Queue(2)
+ process = PopenWorker()
Review comment:
Should this multiprocessing also be changed to PopenPoolWorker? This change is similar to #7889
##########
File path: python/tvm/auto_scheduler/utils.py
##########
@@ -306,6 +307,7 @@ def _func_wrapper(que, func, args, kwargs, add_thread_wrapper):
def call_func_with_timeout(timeout, func, args=(), kwargs=None, add_thread_wrapper=False):
"""Call a function with timeout"""
que = multiprocessing.Queue(2)
+ process = PopenWorker()
Review comment:
Should this multiprocessing also be changed to PopenWorker? This change is similar to #7889
##########
File path: python/tvm/autotvm/record.py
##########
@@ -230,7 +231,7 @@ def split_workload(in_file, clean=True):
lines = list(open(in_file).readlines())
logger.info("start converting...")
- pool = multiprocessing.Pool()
+ pool = popen_pool.PopenPoolExecutor()
Review comment:
No number of threads/processes specified here.
##########
File path: python/tvm/autotvm/tuner/xgboost_cost_model.py
##########
@@ -161,7 +162,8 @@ def _reset_pool(self, space, target, task):
_extract_space = space
_extract_target = target
_extract_task = task
- self.pool = multiprocessing.Pool(self.num_threads)
+ self.pool = popen_pool.PopenPoolExecutor(self.num_threads)
Review comment:
`multiprocessing.pool()` seems to create a process pool instead of a thread pool. Hence the name of the argument passed here `self.num_threads` may not be the most accurate. Is that the case?
##########
File path: python/tvm/autotvm/utils.py
##########
@@ -111,7 +112,7 @@ def pool_map(func, args, batch_size, verbose=False, pool=None):
ret = None
tic = time.time()
- local_pool = pool or multiprocessing.Pool()
+ local_pool = pool or PopenPoolExecutor()
Review comment:
same as above. No specific number of processes here. We may consider adding a default number of process getting created in the implementation of `PopenPoolExecutor`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-912776403
@Wanger-SJTU @echuraev can you check if this fix works? https://github.com/vinx13/tvm/commit/23c5ffce78510a27331eab7561f5586749d0449a
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r673350211
##########
File path: python/tvm/autotvm/tuner/xgboost_cost_model.py
##########
@@ -161,7 +162,8 @@ def _reset_pool(self, space, target, task):
_extract_space = space
_extract_target = target
_extract_task = task
- self.pool = multiprocessing.Pool(self.num_threads)
+ self.pool = popen_pool.PopenPoolExecutor(self.num_threads)
+ # self.pool = multiprocessing.Pool(self.num_threads)
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev commented on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
echuraev commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-912558170
@tqchen, @vinx13 I tried to do that (passing callable function as an argument) and specially I commited these changes on my local branch: https://github.com/echuraev/tvm/tree/echuraev/fix_measure_build_func
But it doesn't work. I got the following error:
```
Get 64 programs to measure:
Traceback (most recent call last):
File "exp_nn/tune_my_model.py", line 148, in <module>
runner()
File "exp_nn/tune_my_model.py", line 143, in runner
run_tuning(tasks, task_weights, log_file)
File "exp_nn/tune_my_model.py", line 93, in run_tuning
tuner.tune(tune_option)
File "/Users/echuraev/Workspace/OctoML/tvm/python/tvm/auto_scheduler/task_scheduler.py", line 357, in tune
self._tune_task(idx)
File "/Users/echuraev/Workspace/OctoML/tvm/python/tvm/auto_scheduler/task_scheduler.py", line 452, in _tune_task
measure_inputs, measure_results = self.search_policies[task_idx].continue_search_one_round(
File "/Users/echuraev/Workspace/OctoML/tvm/python/tvm/auto_scheduler/search_policy.py", line 119, in continue_search_one_round
return _ffi_api.SearchPolicyContinueSearchOneRound(self, num_measure, measurer)
File "/Users/echuraev/Workspace/OctoML/tvm/python/tvm/_ffi/_ctypes/packed_func.py", line 237, in __call__
raise get_last_ffi_error()
ValueError: Traceback (most recent call last):
[bt] (8) 9 libtvm.dylib 0x000000011d4eb681 tvm::runtime::TVMRetValue tvm::runtime::PackedFunc::operator()<tvm::runtime::Array<tvm::auto_scheduler::MeasureInput, void> const&, int&, int&, tvm::runtime::PackedFunc&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >&, int&>(tvm::runtime::Array<tvm::auto_scheduler::
MeasureInput, void> const&, int&, int&, tvm::runtime::PackedFunc&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >&, int&) const + 401
[bt] (7) 8 libtvm.dylib 0x000000011d447b71 std::__1::function<void (tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*)>::operator()(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*) const + 65
[bt] (6) 7 libtvm.dylib 0x000000011d447d9f std::__1::__function::__value_func<void (tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*)>::operator()(tvm::runtime::TVMArgs&&, tvm::runtime::TVMRetValue*&&) const + 95
[bt] (5) 6 libtvm.dylib 0x000000011f707d86 std::__1::__function::__func<TVMFuncCreateFromCFunc::$_2, std::__1::allocator<TVMFuncCreateFromCFunc::$_2>, void (tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*)>::operator()(tvm::runtime::TVMArgs&&, tvm::runtime::TVMRetValue*&&) + 70
[bt] (4) 5 libtvm.dylib 0x000000011f7092c7 std::__1::__function::__alloc_func<TVMFuncCreateFromCFunc::$_2, std::__1::allocator<TVMFuncCreateFromCFunc::$_2>, void (tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*)>::operator()(tvm::runtime::TVMArgs&&, tvm::runtime::TVMRetValue*&&) + 71
[bt] (3) 4 libtvm.dylib 0x000000011f709317 void std::__1::__invoke_void_return_wrapper<void, true>::__call<TVMFuncCreateFromCFunc::$_2&, tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*>(TVMFuncCreateFromCFunc::$_2&, tvm::runtime::TVMArgs&&, tvm::runtime::TVMRetValue*&&) + 71
[bt] (2) 3 libtvm.dylib 0x000000011f7093b3 decltype(std::__1::forward<TVMFuncCreateFromCFunc::$_2&>(fp)(std::__1::forward<tvm::runtime::TVMArgs>(fp0), std::__1::forward<tvm::runtime::TVMRetValue*>(fp0))) std::__1::__invoke<TVMFuncCreateFromCFunc::$_2&, tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*>(TVMFuncCreateFromCFunc::$_2&, tvm::runtime::T
VMArgs&&, tvm::runtime::TVMRetValue*&&) + 115
[bt] (1) 2 libtvm.dylib 0x000000011f70947f TVMFuncCreateFromCFunc::$_2::operator()(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*) const + 175
[bt] (0) 1 libtvm.dylib 0x000000011f74cb55 tvm::runtime::Backtrace() + 37
File "/Users/echuraev/Workspace/OctoML/tvm/python/tvm/_ffi/_ctypes/packed_func.py", line 81, in cfun
rv = local_pyfunc(*pyargs)
File "/Users/echuraev/Workspace/OctoML/tvm/python/tvm/auto_scheduler/measure.py", line 697, in local_builder_build
for res in tuple_res:
File "/Users/echuraev/opt/anaconda3/lib/python3.8/concurrent/futures/_base.py", line 611, in result_iterator
yield fs.pop().result()
File "/Users/echuraev/opt/anaconda3/lib/python3.8/concurrent/futures/_base.py", line 439, in result
return self.__get_result()
File "/Users/echuraev/opt/anaconda3/lib/python3.8/concurrent/futures/_base.py", line 388, in __get_result
raise self._exception
File "/Users/echuraev/opt/anaconda3/lib/python3.8/concurrent/futures/thread.py", line 57, in run
result = self.fn(*self.args, **self.kwargs)
File "/Users/echuraev/Workspace/OctoML/tvm/python/tvm/contrib/popen_pool.py", line 390, in <lambda>
worker = lambda x: self._worker_run_with_error_catching(fn, (x,), None)
File "/Users/echuraev/Workspace/OctoML/tvm/python/tvm/contrib/popen_pool.py", line 343, in _worker_run_with_error_catching
return MapResult(status=StatusKind.COMPLETE, value=self._worker_run(fn, args, kwargs))
File "/Users/echuraev/Workspace/OctoML/tvm/python/tvm/contrib/popen_pool.py", line 338, in _worker_run
proc.send(fn, args, kwargs, self._timeout)
File "/Users/echuraev/Workspace/OctoML/tvm/python/tvm/contrib/popen_pool.py", line 224, in send
data = cloudpickle.dumps((fn, args, kwargs, timeout), protocol=pickle.HIGHEST_PROTOCOL)
File "/Users/echuraev/opt/anaconda3/lib/python3.8/site-packages/cloudpickle/cloudpickle_fast.py", line 73, in dumps
cp.dump(obj)
File "/Users/echuraev/opt/anaconda3/lib/python3.8/site-packages/cloudpickle/cloudpickle_fast.py", line 563, in dump
return Pickler.dump(self, obj)
ValueError: ctypes objects containing pointers cannot be pickled
```
One interesting thing that I tried to dump each of these parameters in tuple `(fn, args, kwargs, timeout)` separately and I didn't see this error. I suppose that the problem is happened because `fn` is a `PackedFunction` with python function. And CloudPickle is not able to serialize native `PackedFunction` object.
Another idea how we can quick fix this problem: move `tar` and `ndk` functions to the `_local_build_worker` and pass only names "tar" or "ndk" to it and select implementation there. In this case it would work for current tvm defined custom functions. In case when passed name is not "tar" or "ndk", then we will have another argument with callable function. It won't work now. Because of this problem with packed functions but we can fix it in the next PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-911854243
@shingjan @vinx13 would you guys take a closer look at the ndk issue as well?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev commented on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
echuraev commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-913333848
> @Wanger-SJTU @echuraev can you check if this fix works? [vinx13@23c5ffc](https://github.com/vinx13/tvm/commit/23c5ffce78510a27331eab7561f5586749d0449a)
I checked, and yes, it works! Thank you very much!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 edited a comment on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 edited a comment on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-911896677
Python function is pickled by it's qualified name (that means the module of the function should be importable). It might be different in our case I'll double check
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-891428035
To convert `tvm.testing` into a sub package, we need to create a `__init__.py` under `testing/` directory, move `testing.py` to `testing/utils.py` and import it in `__init__.py` by adding `from .utils import *`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-912586794
The finding looks correct. Only python function can be sent. PackedFunc cannot be pickled. But they can be looked up by name they are also registered in the worker
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev removed a comment on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
echuraev removed a comment on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-913333848
> @Wanger-SJTU @echuraev can you check if this fix works? [vinx13@23c5ffc](https://github.com/vinx13/tvm/commit/23c5ffce78510a27331eab7561f5586749d0449a)
I checked, and yes, it works! Thank you very much!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r682081304
##########
File path: python/tvm/contrib/popen_pool.py
##########
@@ -269,9 +269,14 @@ class PopenPoolExecutor:
timeout : float
Timeout value for each function submit.
+ Note
+ ----
+ If max_workers is NONE then the number returned by
+ os.cpu_count() is used. This method aligns with the
+ behavior of multiprocessing.pool().
"""
- def __init__(self, max_workers, timeout=None):
+ def __init__(self, max_workers=os.cpu_count(), timeout=None):
Review comment:
this is to make default behavior consistent with `multiprocessing.pool`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r682179140
##########
File path: python/tvm/testing/__init__.py
##########
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=redefined-builtin, wildcard-import
+"""Utility Python functions for TVM testing"""
+from utils import *
Review comment:
One caveat of wildcard import is that it ignores names starting with underscore. We have [a few places](https://github.com/apache/tvm/blob/main/conftest.py#L35) outside the package that these names are used, wildcard here will not work
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 merged pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 merged pull request #8492:
URL: https://github.com/apache/tvm/pull/8492
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r685589705
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -62,6 +63,7 @@
deserialize_workload_registry_entry,
)
+
Review comment:
done
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -817,21 +819,63 @@ def prepare_input_map(args):
return tensor_input_map
+def prepare_runner_args(inp, build_res):
+ """This function prepare the arguments for local/rpc runner in main process
+ Parameters
+ ----------
+ inp : MeasureInput
+ Measure input to be measured.
+
+ build_res : BuildResult
+ Build result to be measured.
+
+ Returns
+ -------
+ List[NDArray, None] :
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-896361626
Please fix the lint
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r672729908
##########
File path: python/tvm/autotvm/tuner/xgboost_cost_model.py
##########
@@ -161,7 +162,8 @@ def _reset_pool(self, space, target, task):
_extract_space = space
_extract_target = target
_extract_task = task
- self.pool = multiprocessing.Pool(self.num_threads)
+ self.pool = popen_pool.PopenPoolExecutor(self.num_threads)
Review comment:
`multiprocessing.pool()` seems to create a process pool instead of a thread pool. Hence the name of the argument passed here `self.num_threads` may not be the most accurate. Is that the case?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev commented on pull request #8492: [AutoScheduler] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
echuraev commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-913341371
> @Wanger-SJTU @echuraev can you check if this fix works? [vinx13@23c5ffc](https://github.com/vinx13/tvm/commit/23c5ffce78510a27331eab7561f5586749d0449a)
I checked, and yes, it works! Thank you very much!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r675215414
##########
File path: python/tvm/autotvm/record.py
##########
@@ -230,7 +231,7 @@ def split_workload(in_file, clean=True):
lines = list(open(in_file).readlines())
logger.info("start converting...")
- pool = multiprocessing.Pool()
+ pool = popen_pool.PopenPoolExecutor()
Review comment:
A default value for number of processes used in `PopenPoolExecutor` is added.
##########
File path: python/tvm/autotvm/utils.py
##########
@@ -111,7 +112,7 @@ def pool_map(func, args, batch_size, verbose=False, pool=None):
ret = None
tic = time.time()
- local_pool = pool or multiprocessing.Pool()
+ local_pool = pool or PopenPoolExecutor()
Review comment:
Same as above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r687937536
##########
File path: python/tvm/auto_scheduler/measure.py
##########
@@ -817,21 +818,66 @@ def prepare_input_map(args):
return tensor_input_map
+def prepare_runner_args(inp, build_res):
+ """This function prepares the pre-defined arguments in `TASK_INPUT_BUFFER_TABLE` for local/rpc
+ runner in main process
+
+ Parameters
+ ----------
+ inp : MeasureInput
+ Measure input to be measured.
+
+ build_res : BuildResult
+ Build result to be measured.
+
+ Returns
+ -------
+ List[Optional[numpy.ndarray]] :
+ List of arguments for running the program. If the argument does not have a pre-defined input
+ buffer, None is added to the list as a placeholder.
+
+ """
+ # pylint: disable=import-outside-toplevel
+ from .search_task import get_task_input_buffer # lazily import to avoid recursive dependency
+
+ task_input_names = inp.task.task_input_names
+ tensor_input_map = prepare_input_map(build_res.args)
+ if not task_input_names:
+ tensor_input_map = {}
+ args = []
+ task_inputs_count = 0
+ for arg in build_res.args:
+ if arg in tensor_input_map:
+ tensor_name = tensor_input_map[arg]
+ if tensor_name in task_input_names:
+ task_input_buffer = get_task_input_buffer(inp.task.workload_key, tensor_name)
+ # convert tvm.NDArray to picklable numpy.ndarray
+ args.append(task_input_buffer.numpy())
Review comment:
It indeed adds some overhead, but benchmark shows this is fine
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r675226836
##########
File path: python/tvm/auto_scheduler/utils.py
##########
@@ -314,6 +314,8 @@ def call_func_with_timeout(timeout, func, args=(), kwargs=None, add_thread_wrapp
except Exception as ex:
res = ex
+ process.kill()
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-896274323
I have checked the rpc runner, since there are already a thread pool layer [here](https://github.com/apache/tvm/blob/main/python/tvm/auto_scheduler/measure.py#L1262) it won't cause too much overhead
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-896232265
Hey thanks @shingjan @vinx13 for finally getting the PR done :-) It is really big improvement that we enable PopenPool so that things could potentially run smoothly in Jupyter. I would love to scrutinize here a little bit on the performance side: @vinx13 please make sure there is no performance degradation on many-core CPUs :-) Thanks a lot!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
vinx13 commented on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-881783963
We can keep the thread pool here, and only replace `multiprocessing.Pool`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 edited a comment on pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 edited a comment on pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#issuecomment-892819421
@vinx13 Would you like to take some extra caution, because I am not sure our test coverage is good enough, try if local/rpc runner of both AutoTVM and Ansor could run smoothly before approving the PR? Thanks a lot 🙏
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #8492: [TIR] Use PopenPool instead of multiprocessing.pool
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #8492:
URL: https://github.com/apache/tvm/pull/8492#discussion_r682113212
##########
File path: python/tvm/testing/utils.py
##########
@@ -0,0 +1,1290 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=invalid-name,unnecessary-comprehension
+""" TVM testing utilities
Review comment:
Okay let's make sure there is a new PR coming soon that removes those copy-paste.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org