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 2020/11/03 23:17:29 UTC

[GitHub] [incubator-tvm] comaniac opened a new pull request #6842: [AutoScheduler] Make SearchTask and ComputeDAG serializable

comaniac opened a new pull request #6842:
URL: https://github.com/apache/incubator-tvm/pull/6842


   This PR adds required `__setstate__` and `__getstate__` to make `SearchTask` and `ComputeDAG` serializable. The idea is to serialize the list of tensors or the schedule used to construct a ComputeDAG, so that we can create an identical ComputeDAG by `pickle.loads`.
   
   Notes
   
   - `import pickle` are lazy imports to avoid unnecessary dependency.
   - We need to add `pickle` to the docker file and update the image to pass the CI.
   
   cc @merrymercy @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.

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



[GitHub] [incubator-tvm] merrymercy merged pull request #6842: [AutoScheduler] Make SearchTask and ComputeDAG serializable

Posted by GitBox <gi...@apache.org>.
merrymercy merged pull request #6842:
URL: https://github.com/apache/incubator-tvm/pull/6842


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] comaniac commented on a change in pull request #6842: [AutoScheduler] Make SearchTask and ComputeDAG serializable

Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #6842:
URL: https://github.com/apache/incubator-tvm/pull/6842#discussion_r519199787



##########
File path: python/tvm/auto_scheduler/compute_dag.py
##########
@@ -182,3 +187,11 @@ def hash_key(self):
 
         str_key = str_key.encode(encoding="utf-8")
         return hashlib.md5(str_key).hexdigest()
+
+    def __getstate__(self):
+        return {"compute": SaveJSON(self.compute), "sche": SaveJSON(self.sche)}
+
+    def __setstate__(self, state):
+        self.compute = LoadJSON(state["compute"])  # pylint: disable=assignment-from-no-return

Review comment:
       We don't call them in SearchTask because all its members have __setstate__ and __getstate__ implemented correctly. This function provides the correct implementation for ConouteDAG and it will be called when processing ComputeDAG in SearchTask.
   
   For Load/SaveJSON here, alternatively we can use pickle.loads/dumps and let them call Load/SaveJSON via Objects. However, this will introduce the dependency of pickle in this function, and will somehow trigger a bug in the unit test. You can test it by replacing Load/SaveJSON with pickle calls. In the unit test I add, you will find that after loading the DAG back, the placeholder A appears twice.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] merrymercy commented on a change in pull request #6842: [AutoScheduler] Make SearchTask and ComputeDAG serializable

Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #6842:
URL: https://github.com/apache/incubator-tvm/pull/6842#discussion_r519157759



##########
File path: python/tvm/auto_scheduler/compute_dag.py
##########
@@ -182,3 +187,11 @@ def hash_key(self):
 
         str_key = str_key.encode(encoding="utf-8")
         return hashlib.md5(str_key).hexdigest()
+
+    def __getstate__(self):
+        return {"compute": SaveJSON(self.compute), "sche": SaveJSON(self.sche)}
+
+    def __setstate__(self, state):
+        self.compute = LoadJSON(state["compute"])  # pylint: disable=assignment-from-no-return

Review comment:
       @comaniac Do we need to explicitly call `SaveJSON`, `LoadJSON` here?
   I find that you don't call these functions in the `SearchTask::__getstate__`




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] comaniac commented on a change in pull request #6842: [AutoScheduler] Make SearchTask and ComputeDAG serializable

Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #6842:
URL: https://github.com/apache/incubator-tvm/pull/6842#discussion_r519199787



##########
File path: python/tvm/auto_scheduler/compute_dag.py
##########
@@ -182,3 +187,11 @@ def hash_key(self):
 
         str_key = str_key.encode(encoding="utf-8")
         return hashlib.md5(str_key).hexdigest()
+
+    def __getstate__(self):
+        return {"compute": SaveJSON(self.compute), "sche": SaveJSON(self.sche)}
+
+    def __setstate__(self, state):
+        self.compute = LoadJSON(state["compute"])  # pylint: disable=assignment-from-no-return

Review comment:
       We don't call them in SearchTask because all its members have __setstate__ and __getstate__ implemented correctly. This function provides the correct implementation for ConouteDAG and it will be called When processing ComputeDAG in SearchTask.
   
   For Load/SaveJSON here, alternatively we can use pickle.loads/dumps and let them call Load/SaveJSON via Objects. However, this will introduce the dependency of pickle in this function, and will somehow trigger a bug in the unit test. You can test it by replacing Load/SaveJSON with pickle calls. In the unit test I add, you will find that after loading the DAG back, the placeholder A appears twice.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] comaniac commented on a change in pull request #6842: [AutoScheduler] Make SearchTask and ComputeDAG serializable

Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #6842:
URL: https://github.com/apache/incubator-tvm/pull/6842#discussion_r519199787



##########
File path: python/tvm/auto_scheduler/compute_dag.py
##########
@@ -182,3 +187,11 @@ def hash_key(self):
 
         str_key = str_key.encode(encoding="utf-8")
         return hashlib.md5(str_key).hexdigest()
+
+    def __getstate__(self):
+        return {"compute": SaveJSON(self.compute), "sche": SaveJSON(self.sche)}
+
+    def __setstate__(self, state):
+        self.compute = LoadJSON(state["compute"])  # pylint: disable=assignment-from-no-return

Review comment:
       We don't call them in SearchTask because all its members have these two functions implemented correctly. This function provides the correct implementation for ConouteDAG and it will be called When processing ComputeDAG in SearchTask.
   
   For Load/SaveJSON here, alternatively we can use pickle.loads/dumps and let them call Load/SaveJSON via Objects. However, this will introduce the dependency of pickle in this function, and will somehow trigger a bug in the unit test. You can test it by replacing Load/SaveJSON with pickle calls. In the unit test I add, you will find that after loading the DAG back, the placeholder A appears twice.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [incubator-tvm] merrymercy commented on a change in pull request #6842: [AutoScheduler] Make SearchTask and ComputeDAG serializable

Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #6842:
URL: https://github.com/apache/incubator-tvm/pull/6842#discussion_r519157759



##########
File path: python/tvm/auto_scheduler/compute_dag.py
##########
@@ -182,3 +187,11 @@ def hash_key(self):
 
         str_key = str_key.encode(encoding="utf-8")
         return hashlib.md5(str_key).hexdigest()
+
+    def __getstate__(self):
+        return {"compute": SaveJSON(self.compute), "sche": SaveJSON(self.sche)}
+
+    def __setstate__(self, state):
+        self.compute = LoadJSON(state["compute"])  # pylint: disable=assignment-from-no-return

Review comment:
       @comaniac Do we need to explicitly call `SaveJSON`, `LoadJSON` here?
   I find that you don't call it in the `SearchTask::__getstate__`




----------------------------------------------------------------
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:
users@infra.apache.org