[GitHub] [tvm] FrozenGene commented on a change in pull request #7185: [AutoScheduler] Add custom build function

2021-01-04 Thread GitBox 


FrozenGene commented on a change in pull request #7185:
URL: https://github.com/apache/tvm/pull/7185#discussion_r551678095



##
File path: python/tvm/auto_scheduler/measure.py
##
@@ -303,12 +315,28 @@ class LocalBuilder(ProgramBuilder):
 This is used in a wrapper of the multiprocessing.Process.join().
 n_parallel : int = multiprocessing.cpu_count()
 Number of threads used to build in parallel.
-build_func : str = 'default'
-The name of registered build function.
+build_func: callable or str = "default"
+If is 'default', use default build function
+If is 'ndk', use function for android ndk
+If is callable, use it as custom build function, expect lib_format 
field.
 """
 
 def __init__(self, timeout=15, n_parallel=multiprocessing.cpu_count(), 
build_func="default"):
-self.__init_handle_by_constructor__(_ffi_api.LocalBuilder, timeout, 
n_parallel, build_func)
+if build_func == "default":
+BuildFunc.name = "default"
+BuildFunc.build_func = tar.tar
+elif build_func == "ndk":
+BuildFunc.name = "ndk"
+BuildFunc.build_func = ndk.create_shared
+elif not isinstance(build_func, str):
+BuildFunc.name = "custom"
+BuildFunc.build_func = build_func

Review comment:
   Sorry, could we modify a little logic here? because we will not enter 
into the `else` branch if we use this condition. For example, we should use 
`elif callable(build_func):`... 





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [tvm] FrozenGene commented on a change in pull request #7185: [AutoScheduler] Add custom build function

2021-01-04 Thread GitBox 


FrozenGene commented on a change in pull request #7185:
URL: https://github.com/apache/tvm/pull/7185#discussion_r551215468



##
File path: python/tvm/auto_scheduler/measure.py
##
@@ -624,12 +652,10 @@ def local_build_worker(args):
 The build result of this Builder thread.
 """
 inp, build_func, timeout, verbose = args
-if build_func == "default":
-build_func = tar.tar
-elif build_func == "ndk":
-build_func = ndk.create_shared
-else:
-raise ValueError("Invalid build_func" + build_func)
+assert build_func == BuildFunc.name, (
+"BuildFunc.name: " + BuildFunc.name + ", but args is: " + build_func
+)
+build_func = BuildFunc.build_func

Review comment:
   i think just pass the custom function name doesn't have much benefit. We 
could just pass
   ```python
   use_ndk = True
   build_func = cc.cross_compiler(ndk.create_shared, options=ndk_options)
   tuner = auto_scheduler.TaskScheduler(tasks, task_weights)
   tune_option = auto_scheduler.TuningOptions(
   num_measure_trials=200, # change this to 2 to achieve the best 
performance
   builder=auto_scheduler.LocalBuilder(build_func=build_func if use_ndk 
else "default"),
   runner=auto_scheduler.RPCRunner(
   device_key, host=tracker_ip, port=tracker_port, repeat=3, timeout=50
   ),
   measure_callbacks=[auto_scheduler.RecordToFile(log_file)],
   )
   ```





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [tvm] FrozenGene commented on a change in pull request #7185: [AutoScheduler] Add custom build function

2021-01-04 Thread GitBox 


FrozenGene commented on a change in pull request #7185:
URL: https://github.com/apache/tvm/pull/7185#discussion_r551167353



##
File path: python/tvm/auto_scheduler/measure.py
##
@@ -624,12 +652,10 @@ def local_build_worker(args):
 The build result of this Builder thread.
 """
 inp, build_func, timeout, verbose = args
-if build_func == "default":
-build_func = tar.tar
-elif build_func == "ndk":
-build_func = ndk.create_shared
-else:
-raise ValueError("Invalid build_func" + build_func)
+assert build_func == BuildFunc.name, (
+"BuildFunc.name: " + BuildFunc.name + ", but args is: " + build_func
+)
+build_func = BuildFunc.build_func

Review comment:
   I query this piece of code git history , it is related with this pr: 
https://github.com/apache/tvm/pull/6671. seems it is about spawn 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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [tvm] FrozenGene commented on a change in pull request #7185: [AutoScheduler] Add custom build function

2021-01-03 Thread GitBox 


FrozenGene commented on a change in pull request #7185:
URL: https://github.com/apache/tvm/pull/7185#discussion_r551149875



##
File path: python/tvm/auto_scheduler/measure.py
##
@@ -652,7 +652,6 @@ def local_build_worker(args):
 The build result of this Builder thread.
 """
 inp, build_func, timeout, verbose = args
-assert any(build_func == name for name in BuildFunc.name)

Review comment:
   it is removed by `black` tool automatically?





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [tvm] FrozenGene commented on a change in pull request #7185: [AutoScheduler] Add custom build function

2021-01-03 Thread GitBox 


FrozenGene commented on a change in pull request #7185:
URL: https://github.com/apache/tvm/pull/7185#discussion_r551141295



##
File path: python/tvm/auto_scheduler/measure.py
##
@@ -303,12 +312,26 @@ class LocalBuilder(ProgramBuilder):
 This is used in a wrapper of the multiprocessing.Process.join().
 n_parallel : int = multiprocessing.cpu_count()
 Number of threads used to build in parallel.
-build_func : str = 'default'
-The name of registered build function.
+build_func: callable or str = "default"
+If is 'default', use default build function
+If is 'ndk', use function for android ndk
+If is callable, use it as custom build function, expect lib_format 
field.
 """
 
 def __init__(self, timeout=15, n_parallel=multiprocessing.cpu_count(), 
build_func="default"):
-self.__init_handle_by_constructor__(_ffi_api.LocalBuilder, timeout, 
n_parallel, build_func)
+if build_func == "default":

Review comment:
   He should keep the state information of `build_func` between functions, 
so just `self.build_func` can  not meet the requirement because we will create 
one class 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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [tvm] FrozenGene commented on a change in pull request #7185: [AutoScheduler] Add custom build function

2021-01-03 Thread GitBox 


FrozenGene commented on a change in pull request #7185:
URL: https://github.com/apache/tvm/pull/7185#discussion_r551139727



##
File path: python/tvm/auto_scheduler/measure.py
##
@@ -624,12 +647,7 @@ def local_build_worker(args):
 The build result of this Builder thread.
 """
 inp, build_func, timeout, verbose = args
-if build_func == "default":
-build_func = tar.tar
-elif build_func == "ndk":
-build_func = ndk.create_shared
-else:
-raise ValueError("Invalid build_func" + build_func)
+build_func = BuildFunc.build_func

Review comment:
   If this is a custom build func, we can not serialize it into args, 
because it is a callable object, not one str.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [tvm] FrozenGene commented on a change in pull request #7185: [AutoScheduler] Add custom build function

2021-01-03 Thread GitBox 


FrozenGene commented on a change in pull request #7185:
URL: https://github.com/apache/tvm/pull/7185#discussion_r551139319



##
File path: python/tvm/auto_scheduler/measure.py
##
@@ -624,12 +647,7 @@ def local_build_worker(args):
 The build result of this Builder thread.
 """
 inp, build_func, timeout, verbose = args
-if build_func == "default":
-build_func = tar.tar
-elif build_func == "ndk":
-build_func = ndk.create_shared
-else:
-raise ValueError("Invalid build_func" + build_func)

Review comment:
   we could add one check `assert any(build_func == name for name in  
BuildFunc.nam)`

##
File path: python/tvm/auto_scheduler/measure.py
##
@@ -63,6 +63,15 @@
 # We use 1e10 instead of sys.float_info.max for better readability in log
 MAX_FLOAT = 1e10
 
+class BuildFunc:
+""" store build_func name and callable to class variable.

Review comment:
   No need of `to class variable`





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [tvm] FrozenGene commented on a change in pull request #7185: [AutoScheduler] Add custom build function

2020-12-30 Thread GitBox 


FrozenGene commented on a change in pull request #7185:
URL: https://github.com/apache/tvm/pull/7185#discussion_r550388292



##
File path: python/tvm/auto_scheduler/measure.py
##
@@ -303,11 +306,16 @@ class LocalBuilder(ProgramBuilder):
 This is used in a wrapper of the multiprocessing.Process.join().
 n_parallel : int = multiprocessing.cpu_count()
 Number of threads used to build in parallel.
-build_func : str = 'default'
-The name of registered build function.
+build_func: callable or str

Review comment:
   add the default value in the comment

##
File path: python/tvm/auto_scheduler/measure.py
##
@@ -628,6 +636,8 @@ def local_build_worker(args):
 build_func = tar.tar
 elif build_func == "ndk":
 build_func = ndk.create_shared
+elif build_func == "custom":

Review comment:
   if so, we could use BuildFunc.build_func for all conditions

##
File path: python/tvm/auto_scheduler/measure.py
##
@@ -63,6 +63,9 @@
 # We use 1e10 instead of sys.float_info.max for better readability in log
 MAX_FLOAT = 1e10
 
+class CustomBuildFunc:

Review comment:
   Suggest we use one class BuildFunc, which could store default / ndk / 
custom and so on.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org