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/06/02 17:08:47 UTC
[GitHub] [incubator-tvm] notoraptor opened a new pull request #5716: [topi][relay] Add operation gather to relay.
notoraptor opened a new pull request #5716:
URL: https://github.com/apache/incubator-tvm/pull/5716
Hi! This is a PR to add operation `gather` to relay, the same operation as in torch: https://pytorch.org/docs/stable/torch.html#torch.gather
----------------------------------------------------------------
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] tqchen commented on pull request #5716: [topi][relay] Add operation gather to relay.
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5716:
URL: https://github.com/apache/incubator-tvm/pull/5716#issuecomment-643333642
Thanks @notoraptor for contribution! Thanks @abergeron @mbrookhart for reviewing!
----------------------------------------------------------------
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] notoraptor commented on pull request #5716: [topi][relay] Add operation gather to relay.
Posted by GitBox <gi...@apache.org>.
notoraptor commented on pull request #5716:
URL: https://github.com/apache/incubator-tvm/pull/5716#issuecomment-642209115
Rebased to fix conflicts. @tqchen PR is approved by @abergeron !
----------------------------------------------------------------
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] notoraptor commented on a change in pull request #5716: [topi][relay] Add operation gather to relay.
Posted by GitBox <gi...@apache.org>.
notoraptor commented on a change in pull request #5716:
URL: https://github.com/apache/incubator-tvm/pull/5716#discussion_r434051229
##########
File path: topi/include/topi/transform.h
##########
@@ -982,6 +982,54 @@ inline Tensor tile(const Tensor& x, Array<Integer> reps, std::string name = "T_t
}
}
+/*!
+ * \brief Gather values along given axis from given indices.
+ *
+ * \param data The input data to the operator.
+ * \param axis The axis along which to index.
+ * \param indices The indices of values to gather.
+ * \param name The name of the operation.
+ * \param tag The tag to mark the operation.
+ *
+ * \return A Tensor whose op member is the gather_nd operation
Review comment:
Fixed
##########
File path: topi/python/topi/testing/gather_python.py
##########
@@ -0,0 +1,46 @@
+# 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, line-too-long, unused-variable, too-many-locals
+"""gather_nd in python"""
Review comment:
Fixed
----------------------------------------------------------------
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] abergeron commented on a change in pull request #5716: [topi][relay] Add operation gather to relay.
Posted by GitBox <gi...@apache.org>.
abergeron commented on a change in pull request #5716:
URL: https://github.com/apache/incubator-tvm/pull/5716#discussion_r434044772
##########
File path: topi/include/topi/transform.h
##########
@@ -982,6 +982,54 @@ inline Tensor tile(const Tensor& x, Array<Integer> reps, std::string name = "T_t
}
}
+/*!
+ * \brief Gather values along given axis from given indices.
+ *
+ * \param data The input data to the operator.
+ * \param axis The axis along which to index.
+ * \param indices The indices of values to gather.
+ * \param name The name of the operation.
+ * \param tag The tag to mark the operation.
+ *
+ * \return A Tensor whose op member is the gather_nd operation
Review comment:
You probably mean
```suggestion
* \return A Tensor whose op member is the gather operation
```
##########
File path: topi/python/topi/testing/gather_python.py
##########
@@ -0,0 +1,46 @@
+# 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, line-too-long, unused-variable, too-many-locals
+"""gather_nd in python"""
Review comment:
```suggestion
"""gather in python"""
```
----------------------------------------------------------------
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] tqchen commented on pull request #5716: [topi][relay] Add operation gather to relay.
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5716:
URL: https://github.com/apache/incubator-tvm/pull/5716#issuecomment-639776043
@abergeron please https://tvm.apache.org/docs/contribute/code_review.html#approve-and-request-changes-explicitly
----------------------------------------------------------------
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] tqchen merged pull request #5716: [topi][relay] Add operation gather to relay.
Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #5716:
URL: https://github.com/apache/incubator-tvm/pull/5716
----------------------------------------------------------------
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] notoraptor commented on pull request #5716: [topi][relay] Add operation gather to relay.
Posted by GitBox <gi...@apache.org>.
notoraptor commented on pull request #5716:
URL: https://github.com/apache/incubator-tvm/pull/5716#issuecomment-639035230
@mbrookhart Hi ! There is a difference at least in the output shape for take: https://github.com/mbrookhart/incubator-tvm/blob/062a244d66262353cdef0792a54d05cc99d7fa74/topi/include/topi/transform.h#L762 .
With `take`, in output shape, the axis seems to be replaced with indices size. So, for example, if data has shape `(1, 2, 3)`, and axis is `1`, and indices has shape `(4, 5)`, then output shape will be `(1, 4, 5, 3)`.
With the `gather` operation here, number of dimensions does not change as output shape only replaces axis with corresponding axis shape in indices (so that output has same shape as indices). For example, if data has shape `(1, 2, 3)`, and axis is `1`, and indices has shape `(1, 7, 3)` (indices must have same ndim as input), then output shape will be `(1, 7, 3)`.
I may look for other existing operations which would allow to get same result as this `gather`, but it seems I can not achieve it with `take`. Each take-like operation seems to come with its own deails.
----------------------------------------------------------------
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] tqchen commented on pull request #5716: [topi][relay] Add operation gather to relay.
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5716:
URL: https://github.com/apache/incubator-tvm/pull/5716#issuecomment-638447821
@mbrookhart @Huyuwei can you help to review the 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org