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/09/09 09:06:19 UTC

[GitHub] [incubator-tvm] jainris opened a new pull request #6429: [Relay/TOPI] Added 'offsets' and 'alignment' attributes to MATRIX_SET_DIAG.

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


   This is a follow-up PR of #6303.
   
   * Added support for 'offsets' and 'alignment' attributes of MATRIX_SET_DIAG.
     (Similar to MATRIX_SET_DIAG V3 of TF)
   * Added tests for 'offsets' and 'alignment' attributes of MATRIX_SET_DIAG.


----------------------------------------------------------------
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] siju-samuel commented on a change in pull request #6429: [Relay/TOPI] Added 'offsets' and 'alignment' attributes to MATRIX_SET_DIAG.

Posted by GitBox <gi...@apache.org>.
siju-samuel commented on a change in pull request #6429:
URL: https://github.com/apache/incubator-tvm/pull/6429#discussion_r493740837



##########
File path: python/tvm/relay/op/transform.py
##########
@@ -1178,17 +1178,33 @@ def sparse_to_dense(sparse_indices, output_shape, sparse_values, default_value=0
     return _make.sparse_to_dense(sparse_indices, output_shape, sparse_values, default_value)
 
 
-def matrix_set_diag(data, diagonal):
+def matrix_set_diag(data, diagonal, k=0, align="RIGHT_LEFT"):
     """
-    Returns a tensor with the diagonal of input tensor replaced with the provided diagonal values.
+    Returns a tensor with the diagonals of input tensor replaced with the provided diagonal values.
 
     Parameters
     ----------
     data : relay.Expr
         Input Tensor.
+
     diagonal : relay.Expr
         Values to be filled in the diagonal.
 
+    k : int or tuple of int

Review comment:
       add `Optional` for k and align

##########
File path: tests/python/relay/test_op_level10.py
##########
@@ -520,17 +518,18 @@ def _verify(input_shape, dtype):
         func = relay.Function([input, diagonal], out)
         input_np = np.random.randint(-100, 100, size=input_shape).astype(dtype)
         diagonal_np = np.random.randint(-100, 100, size=diagonal_shape).astype(dtype)
-        out_np = tvm.topi.testing.matrix_set_diag(input_np, diagonal_np)
+        out_np = tvm.topi.testing.matrix_set_diag(input_np, diagonal_np, k, align)
 
         for target, ctx in tvm.testing.enabled_targets():
             for kind in ["graph", "debug"]:
                 intrp = relay.create_executor(kind, ctx=ctx, target=target)
                 out_relay = intrp.evaluate(func)(input_np, diagonal_np)
                 tvm.testing.assert_allclose(out_relay.asnumpy(), out_np)
 
-    _verify((2, 2), "float32")
-    _verify((4, 3, 3), "int32")
-    _verify((2, 3, 4), "float32")
+    _verify((2, 2), (2,), "float32")
+    _verify((4, 3, 3), (4, 3), "int32")
+    _verify((2, 3, 4), (2, 3), "float32", 1)
+    _verify((2, 3, 4), (2, 4, 3), "int32", (-1, 2), "LEFT_RIGHT")

Review comment:
       add testcases for  "LEFT_LEFT" & "RIGHT_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.

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



[GitHub] [incubator-tvm] anijain2305 merged pull request #6429: [Relay/TOPI] Added 'offsets' and 'alignment' attributes to MATRIX_SET_DIAG.

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


   


----------------------------------------------------------------
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] jainris commented on pull request #6429: [Relay/TOPI] Added 'offsets' and 'alignment' attributes to MATRIX_SET_DIAG.

Posted by GitBox <gi...@apache.org>.
jainris commented on pull request #6429:
URL: https://github.com/apache/incubator-tvm/pull/6429#issuecomment-698425821


   @anijain2305 I believe this is ready to be 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.

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



[GitHub] [incubator-tvm] anijain2305 commented on a change in pull request #6429: [Relay/TOPI] Added 'offsets' and 'alignment' attributes to MATRIX_SET_DIAG.

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



##########
File path: python/tvm/topi/transform.py
##########
@@ -806,17 +806,33 @@ def sparse_to_dense(sparse_indices, output_shape, sparse_values, default_value=0
     return cpp.sparse_to_dense(sparse_indices, output_shape, sparse_values, default_value)
 
 
-def matrix_set_diag(data, diagonal):
+def matrix_set_diag(data, diagonal, k=0, align="RIGHT_LEFT"):
     """
-    Returns a tensor with the diagonal of input tensor replaced with the provided diagonal values.
+    Returns a tensor with the diagonals of input tensor replaced with the provided diagonal values.
 
     Parameters
     ----------
     data : relay.Expr
         Input Tensor.
+
     diagonal : relay.Expr
         Values to be filled in the diagonal.
 
+    k : int or tuple of int

Review comment:
       Set optional as Samuel mentioned earlier




----------------------------------------------------------------
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] mbaret commented on pull request #6429: [Relay/TOPI] Added 'offsets' and 'alignment' attributes to MATRIX_SET_DIAG.

Posted by GitBox <gi...@apache.org>.
mbaret commented on pull request #6429:
URL: https://github.com/apache/incubator-tvm/pull/6429#issuecomment-697341590


   ping @siju-samuel could you take a look at this as I believe you requested this functionality?


----------------------------------------------------------------
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] anijain2305 commented on pull request #6429: [Relay/TOPI] Added 'offsets' and 'alignment' attributes to MATRIX_SET_DIAG.

Posted by GitBox <gi...@apache.org>.
anijain2305 commented on pull request #6429:
URL: https://github.com/apache/incubator-tvm/pull/6429#issuecomment-698427218


   Thanks @jainris @mbaret @siju-samuel @u99127 This is 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.

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



[GitHub] [incubator-tvm] jainris commented on pull request #6429: [Relay/TOPI] Added 'offsets' and 'alignment' attributes to MATRIX_SET_DIAG.

Posted by GitBox <gi...@apache.org>.
jainris commented on pull request #6429:
URL: https://github.com/apache/incubator-tvm/pull/6429#issuecomment-698425821


   @anijain2305 I believe this is ready to be 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.

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



[GitHub] [incubator-tvm] mbaret commented on a change in pull request #6429: [Relay/TOPI] Added 'offsets' and 'alignment' attributes to MATRIX_SET_DIAG.

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



##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -3131,36 +3133,62 @@ bool MatrixSetDiagRel(const Array<Type>& types, int num_inputs, const Attrs& att
   const auto* diagonal = types[1].as<TensorTypeNode>();
   CHECK(diagonal);
 
+  const auto param = attrs.as<MatrixSetDiagAttrs>();
+  CHECK_GE(param->k2, param->k1);
+

Review comment:
       Could you also add a check to make sure the k values are in a sane range for the input tensor size?

##########
File path: include/tvm/topi/transform.h
##########
@@ -1524,29 +1524,60 @@ inline Tensor sparse_to_dense(const Tensor& sparse_indices, const Array<Integer>
 }
 
 /*!
- * \brief Returns a tensor with the diagonal of input tensor replaced with the provided diagonal.
+ * \brief Returns a tensor with the diagonal of input tensor replaced with the provided diagonals.
  * \param input input tensor.
- * \param diagonal values to be filled in the diagonal.
+ * \param diagonal values to be filled in the diagonals.
+ * \param k1 lower limit (included) of the range of diagonals.
+ * \param k2 upper limit (included) of the range of diagonals.
+ * \param super_diag_right_align bool, true iff super-diagonal is right aligned (left-padded).
+ * \param sub_diag_right_align bool, true iff sub-diagonal is right aligned (left-padded).
  * \param name output tensor name.
  * \param tag output tensor tag.
  * \return new tensor with given diagonal values.
  */
-inline Tensor matrix_set_diag(const Tensor& input, const Tensor& diagonal,
+inline Tensor matrix_set_diag(const Tensor& input, const Tensor& diagonal, int k1, int k2,
+                              bool super_diag_right_align, bool sub_diag_right_align,
                               const std::string name = "T_matrix_set_diag",
                               const std::string tag = kInjective) {
   size_t ndim = input->shape.size() - 1;
 
+  bool onlyOneDiagonal = k1 == k2;

Review comment:
       only_one_diagonal




----------------------------------------------------------------
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] anijain2305 merged pull request #6429: [Relay/TOPI] Added 'offsets' and 'alignment' attributes to MATRIX_SET_DIAG.

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


   


----------------------------------------------------------------
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] anijain2305 commented on pull request #6429: [Relay/TOPI] Added 'offsets' and 'alignment' attributes to MATRIX_SET_DIAG.

Posted by GitBox <gi...@apache.org>.
anijain2305 commented on pull request #6429:
URL: https://github.com/apache/incubator-tvm/pull/6429#issuecomment-698427218


   Thanks @jainris @mbaret @siju-samuel @u99127 This is 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.

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



[GitHub] [incubator-tvm] zhiics commented on pull request #6429: [Relay/TOPI] Added 'offsets' and 'alignment' attributes to MATRIX_SET_DIAG.

Posted by GitBox <gi...@apache.org>.
zhiics commented on pull request #6429:
URL: https://github.com/apache/incubator-tvm/pull/6429#issuecomment-694943836


   @siju-samuel @mbaret @anijain2305 @FrozenGene please help review. Thanks.


----------------------------------------------------------------
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] jainris commented on pull request #6429: [Relay/TOPI] Added 'offsets' and 'alignment' attributes to MATRIX_SET_DIAG.

Posted by GitBox <gi...@apache.org>.
jainris commented on pull request #6429:
URL: https://github.com/apache/incubator-tvm/pull/6429#issuecomment-689434134


   cc @siju-samuel @mbaret @anijain2305 @u99127 @FrozenGene @tqchen


----------------------------------------------------------------
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