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 2022/12/15 18:05:19 UTC
[GitHub] [tvm] shingjan opened a new pull request, #13623: [Relay][Runtime] Add `set_input/output_zero_copy` in python
shingjan opened a new pull request, #13623:
URL: https://github.com/apache/tvm/pull/13623
As titled, this apis is in cpp but not python. Added this apis for GraphModule and some tests.
cc: @vinx13
--
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 #13623: [Relay][Runtime] Add `set_input/output_zero_copy` in python
Posted by GitBox <gi...@apache.org>.
shingjan commented on PR #13623:
URL: https://github.com/apache/tvm/pull/13623#issuecomment-1354175289
@tvm-bot re-run
--
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] tvm-bot commented on pull request #13623: [Relay][Runtime] Add `set_input/output_zero_copy` in python
Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13623:
URL: https://github.com/apache/tvm/pull/13623#issuecomment-1353504164
<!---bot-comment-->
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
<!--bot-comment-ccs-start-->
* cc @areusch <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
<sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>
--
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 a diff in pull request #13623: [Relay][Runtime] Add `set_input/output_zero_copy` in python
Posted by GitBox <gi...@apache.org>.
echuraev commented on code in PR #13623:
URL: https://github.com/apache/tvm/pull/13623#discussion_r1050386160
##########
tests/python/unittest/test_runtime_module_based_interface.py:
##########
@@ -688,6 +688,36 @@ def test_num_threads():
assert reported == hardware_threads or reported == hardware_threads // 2
+@tvm.testing.requires_llvm
+def test_graph_module_zero_copy():
Review Comment:
Probably the second test case can be added. This test case should test setting `zero_input` when the keys and values passed through `params`
--
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] masahi merged pull request #13623: [Relay][Runtime] Add `set_input/output_zero_copy` in python
Posted by GitBox <gi...@apache.org>.
masahi merged PR #13623:
URL: https://github.com/apache/tvm/pull/13623
--
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 diff in pull request #13623: [Relay][Runtime] Add `set_input/output_zero_copy` in python
Posted by GitBox <gi...@apache.org>.
shingjan commented on code in PR #13623:
URL: https://github.com/apache/tvm/pull/13623#discussion_r1050055038
##########
python/tvm/contrib/graph_executor.py:
##########
@@ -195,6 +197,47 @@ def set_input(self, key=None, value=None, **params):
if val:
self._get_input(k).copyfrom(params[k])
+ def set_input_zero_copy(self, key, value, **params):
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 diff in pull request #13623: [Relay][Runtime] Add `set_input/output_zero_copy` in python
Posted by GitBox <gi...@apache.org>.
shingjan commented on code in PR #13623:
URL: https://github.com/apache/tvm/pull/13623#discussion_r1049998823
##########
python/tvm/contrib/graph_executor.py:
##########
@@ -195,6 +198,49 @@ def set_input(self, key=None, value=None, **params):
if val:
self._get_input(k).copyfrom(params[k])
+ def set_output(self, key, value):
+ """Set outputs to the module
+
+ Parameters
+ ----------
+ key : int or str
+ The output key
+
+ value : the output value
+ The output value
+ """
+ self._set_output(key, value)
+
+ def set_input_zero_copy(self, key, value, **params):
+ """Set inputs to the module via kwargs with zero memory copy
+
+ Parameters
+ ----------
+ key : int or str
+ The input key
+
+ value : the input value in DLPack
+ The input key
+
+ params : dict of str to NDArray
+ Additional arguments
+ """
+ self._set_input_zero_copy(key, value)
+ self.set_input(None, None, **params)
Review Comment:
I was hoping that we can keep the apis consistent with `set_input` so that is the reason I kept `**params`. Should we still remove it?
--
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 diff in pull request #13623: [Relay][Runtime] Add `set_input/output_zero_copy` in python
Posted by GitBox <gi...@apache.org>.
vinx13 commented on code in PR #13623:
URL: https://github.com/apache/tvm/pull/13623#discussion_r1050002262
##########
python/tvm/contrib/graph_executor.py:
##########
@@ -195,6 +198,49 @@ def set_input(self, key=None, value=None, **params):
if val:
self._get_input(k).copyfrom(params[k])
+ def set_output(self, key, value):
+ """Set outputs to the module
+
+ Parameters
+ ----------
+ key : int or str
+ The output key
+
+ value : the output value
+ The output value
+ """
+ self._set_output(key, value)
+
+ def set_input_zero_copy(self, key, value, **params):
+ """Set inputs to the module via kwargs with zero memory copy
+
+ Parameters
+ ----------
+ key : int or str
+ The input key
+
+ value : the input value in DLPack
+ The input key
+
+ params : dict of str to NDArray
+ Additional arguments
+ """
+ self._set_input_zero_copy(key, value)
+ self.set_input(None, None, **params)
Review Comment:
We can support `params`. However, we can't call `set_input` as it involves with copying.
--
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 diff in pull request #13623: [Relay][Runtime] Add `set_input/output_zero_copy` in python
Posted by GitBox <gi...@apache.org>.
shingjan commented on code in PR #13623:
URL: https://github.com/apache/tvm/pull/13623#discussion_r1050009039
##########
python/tvm/contrib/graph_executor.py:
##########
@@ -195,6 +198,49 @@ def set_input(self, key=None, value=None, **params):
if val:
self._get_input(k).copyfrom(params[k])
+ def set_output(self, key, value):
+ """Set outputs to the module
+
+ Parameters
+ ----------
+ key : int or str
+ The output key
+
+ value : the output value
+ The output value
+ """
+ self._set_output(key, value)
+
+ def set_input_zero_copy(self, key, value, **params):
+ """Set inputs to the module via kwargs with zero memory copy
+
+ Parameters
+ ----------
+ key : int or str
+ The input key
+
+ value : the input value in DLPack
+ The input key
+
+ params : dict of str to NDArray
+ Additional arguments
+ """
+ self._set_input_zero_copy(key, value)
+ self.set_input(None, None, **params)
Review Comment:
good point. Updated this method to use zero copy.
--
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 diff in pull request #13623: [Relay][Runtime] Add `set_input/output_zero_copy` in python
Posted by GitBox <gi...@apache.org>.
shingjan commented on code in PR #13623:
URL: https://github.com/apache/tvm/pull/13623#discussion_r1051127996
##########
tests/python/unittest/test_runtime_module_based_interface.py:
##########
@@ -688,6 +688,36 @@ def test_num_threads():
assert reported == hardware_threads or reported == hardware_threads // 2
+@tvm.testing.requires_llvm
+def test_graph_module_zero_copy():
Review Comment:
@echuraev a second test case is added for setting params input.
--
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 diff in pull request #13623: [Relay][Runtime] Add `set_input/output_zero_copy` in python
Posted by GitBox <gi...@apache.org>.
vinx13 commented on code in PR #13623:
URL: https://github.com/apache/tvm/pull/13623#discussion_r1050047034
##########
python/tvm/contrib/graph_executor.py:
##########
@@ -195,6 +197,47 @@ def set_input(self, key=None, value=None, **params):
if val:
self._get_input(k).copyfrom(params[k])
+ def set_input_zero_copy(self, key, value, **params):
Review Comment:
```suggestion
def set_input_zero_copy(self, key=None, value=None, **params):
```
--
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 diff in pull request #13623: [Relay][Runtime] Add `set_input/output_zero_copy` in python
Posted by GitBox <gi...@apache.org>.
vinx13 commented on code in PR #13623:
URL: https://github.com/apache/tvm/pull/13623#discussion_r1049990723
##########
src/runtime/graph_executor/graph_executor.cc:
##########
@@ -160,6 +160,16 @@ void GraphExecutor::SetInput(int index, DLTensor* data_in) {
uint32_t eid = this->entry_id(input_nodes_[index], 0);
data_entry_[eid].CopyFrom(data_in);
}
+/*!
+ * \brief set index-th output to the graph.
+ * \param index The output index.
+ * \param data_in The output data.
+ */
+void GraphExecutor::SetOutput(int index, DLTensor* data_in) {
Review Comment:
not needed
##########
python/tvm/contrib/graph_executor.py:
##########
@@ -195,6 +198,49 @@ def set_input(self, key=None, value=None, **params):
if val:
self._get_input(k).copyfrom(params[k])
+ def set_output(self, key, value):
+ """Set outputs to the module
+
+ Parameters
+ ----------
+ key : int or str
+ The output key
+
+ value : the output value
+ The output value
+ """
+ self._set_output(key, value)
Review Comment:
this is not needed, it doesn't make sense to copy external data to the output tensor
##########
python/tvm/contrib/graph_executor.py:
##########
@@ -195,6 +198,49 @@ def set_input(self, key=None, value=None, **params):
if val:
self._get_input(k).copyfrom(params[k])
+ def set_output(self, key, value):
+ """Set outputs to the module
+
+ Parameters
+ ----------
+ key : int or str
+ The output key
+
+ value : the output value
+ The output value
+ """
+ self._set_output(key, value)
+
+ def set_input_zero_copy(self, key, value, **params):
+ """Set inputs to the module via kwargs with zero memory copy
+
+ Parameters
+ ----------
+ key : int or str
+ The input key
+
+ value : the input value in DLPack
+ The input key
+
+ params : dict of str to NDArray
+ Additional arguments
+ """
+ self._set_input_zero_copy(key, value)
+ self.set_input(None, None, **params)
Review Comment:
not needed
--
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 diff in pull request #13623: [Relay][Runtime] Add `set_input/output_zero_copy` in python
Posted by GitBox <gi...@apache.org>.
vinx13 commented on code in PR #13623:
URL: https://github.com/apache/tvm/pull/13623#discussion_r1050014668
##########
python/tvm/contrib/graph_executor.py:
##########
@@ -212,7 +212,17 @@ def set_input_zero_copy(self, key, value, **params):
Additional arguments
"""
self._set_input_zero_copy(key, value)
- self.set_input(None, None, **params)
+ if params:
+ # upload big arrays first to avoid memory issue in rpc mode
+ keys = list(params.keys())
+ keys.sort(key=lambda x: -np.prod(params[x].shape))
Review Comment:
probably not needed as there is no data copy
##########
python/tvm/contrib/graph_executor.py:
##########
@@ -212,7 +212,17 @@ def set_input_zero_copy(self, key, value, **params):
Additional arguments
"""
self._set_input_zero_copy(key, value)
- self.set_input(None, None, **params)
+ if params:
Review Comment:
`key, value` vs `params` are basically two ways to specify the input. if `params` is supported, `key, value` should be by default None (similar to `set_input`)
--
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