[GitHub] [incubator-tvm] icemelon9 commented on a change in pull request #5459: [Relay][Op]Support symbolic TopK, Ones, Zeros and Full

2020-05-26 Thread GitBox


icemelon9 commented on a change in pull request #5459:
URL: https://github.com/apache/incubator-tvm/pull/5459#discussion_r430022484



##
File path: src/relay/analysis/util.cc
##
@@ -450,6 +451,13 @@ bool IsDataDependant(const CallNode* call) {
 return false;
   }
 }
+  } else if (op->name == "topk") {

Review comment:
   We should also apply this to ones/zeros/full/broadcast_to.





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] [incubator-tvm] icemelon9 commented on a change in pull request #5459: [Relay][Op]Support symbolic TopK, Ones, Zeros and Full

2020-05-19 Thread GitBox


icemelon9 commented on a change in pull request #5459:
URL: https://github.com/apache/incubator-tvm/pull/5459#discussion_r427061276



##
File path: src/relay/transforms/pattern_util.h
##
@@ -325,6 +345,65 @@ inline bool IsEqualScalar(const Expr& a, const Expr& b) {
   return tvm::StructuralEqual()(a, b);
 }
 
+/*!
+ * \brief Convert an element of a NDArray with type int or float to scalar.
+ * \param array Input NDArray
+ * \param i element index
+ * \return Converted scalar value.
+ */
+static inline double ToScalar(const runtime::NDArray& array, size_t i = 0) {
+  if (array->dtype.code == kDLInt) {
+if (array->dtype.bits == 8) {
+  return reinterpret_cast(array->data)[i];
+} else if (array->dtype.bits == 16) {
+  return reinterpret_cast(array->data)[i];
+} else if (array->dtype.bits == 32) {
+  return reinterpret_cast(array->data)[i];
+} else if (array->dtype.bits == 64) {
+  return reinterpret_cast(array->data)[i];
+}
+  } else if (array->dtype.code == kDLUInt) {
+if (array->dtype.bits == 8) {
+  return reinterpret_cast(array->data)[i];
+} else if (array->dtype.bits == 16) {
+  return reinterpret_cast(array->data)[i];
+} else if (array->dtype.bits == 32) {
+  return reinterpret_cast(array->data)[i];
+} else if (array->dtype.bits == 64) {
+  return reinterpret_cast(array->data)[i];
+}
+  } else if (array->dtype.code == kDLFloat) {
+#if (__ARM_FP16_FORMAT_IEEE == 1)
+if (array->dtype.bits == 16) {
+  return reinterpret_cast<__fp16*>(array->data)[i];
+}
+#endif
+if (array->dtype.bits == 32) {
+  return reinterpret_cast(array->data)[i];
+} else if (array->dtype.bits == 64) {
+  return reinterpret_cast(array->data)[i];
+}
+  }
+  LOG(FATAL) << "Unknown data type: " << 
tvm::runtime::DLDataType2String(array->dtype);
+  // make compiler happy
+  return -std::numeric_limits::infinity();
+}
+
+/*!
+ * \brief Convert a NDArray with type int or float to Array.
+ * \param array Input NDArray
+ * \return Converted Array.
+ */
+static inline Array ToVector(const runtime::NDArray& array) {
+  size_t len = array.Shape().front();

Review comment:
   Probably check the ndim == 1 here?





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] [incubator-tvm] icemelon9 commented on a change in pull request #5459: [Relay][Op]Support symbolic TopK, Ones, Zeros and Full

2020-05-15 Thread GitBox


icemelon9 commented on a change in pull request #5459:
URL: https://github.com/apache/incubator-tvm/pull/5459#discussion_r425950082



##
File path: src/relay/op/tensor/transform.cc
##
@@ -47,6 +47,66 @@ namespace tvm {
 namespace relay {
 using tir::IntImmNode;
 
+int64_t* ToVector(const runtime::NDArray& array) {
+  size_t len = array.Shape().front();
+  int64_t* rel_vec = new int64_t[len];

Review comment:
   Also consider move this function and [similar 
one](https://github.com/apache/incubator-tvm/blob/master/src/relay/op/tensor/transform.cc#L450)
 to some util.h so that others can reuse.

##
File path: src/relay/op/tensor/transform.cc
##
@@ -944,7 +1005,21 @@ bool FullRel(const Array& types, int num_inputs, 
const Attrs& attrs,
   CHECK_EQ(fill_value->shape.size(), 0)
   << "Fill value should be a scalar but has dimension " << 
fill_value->shape.size() << ".";
 
-  reporter->Assign(types[1], TensorType(param->shape, out_dtype));
+  const IntImmNode* shape_shape = fill_shape->shape[0].as();
+  CHECK(shape_shape) << "Parameter shape must have static shape";
+
+  std::vector oshape;
+  if (param->shape) {
+const Array& cshape_array = param->shape.value();
+for (size_t i = 0; i < cshape_array.size(); ++i) {
+  oshape.push_back(cshape_array[i]);
+}

Review comment:
   I think you can merge line 1013-1016 to `ToVector` function since all 
the use cases need `Array`.

##
File path: src/relay/op/tensor/transform.cc
##
@@ -47,6 +47,66 @@ namespace tvm {
 namespace relay {
 using tir::IntImmNode;
 
+int64_t* ToVector(const runtime::NDArray& array) {
+  size_t len = array.Shape().front();
+  int64_t* rel_vec = new int64_t[len];

Review comment:
   Please don't use raw pointer. it will cause memory leak.

##
File path: topi/python/topi/sort.py
##
@@ -133,7 +133,10 @@ def topk(data, k=1, axis=-1, ret_type="both", 
is_ascend=False, dtype="int64"):
 assert ret_type in ["both", "values", "indices"]
 data_buf = tvm.tir.decl_buffer(data.shape, data.dtype, "data_buf", 
data_alignment=8)
 out_shape = list(get_const_tuple(data.shape))
-if k >= 1:
+kvar = tvm.te.var("k")

Review comment:
   ```suggestion
   kvar = tvm.te.size_var("k")
   ```





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] [incubator-tvm] icemelon9 commented on a change in pull request #5459: [Relay][Op]Support symbolic TopK, Ones, Zeros and Full

2020-04-28 Thread GitBox


icemelon9 commented on a change in pull request #5459:
URL: https://github.com/apache/incubator-tvm/pull/5459#discussion_r417073724



##
File path: include/tvm/relay/attrs/transform.h
##
@@ -110,7 +110,7 @@ struct TakeAttrs : public tvm::AttrsNode {
 
 /*! \brief Attributes that specify a tensor */
 struct InitOpAttrs : public tvm::AttrsNode {
-  Array shape;
+  Expr shape;

Review comment:
   same here.

##
File path: include/tvm/relay/attrs/algorithm.h
##
@@ -50,14 +50,14 @@ struct ArgsortAttrs : public tvm::AttrsNode {
 };
 
 struct TopKAttrs : public tvm::AttrsNode {
-  int k;
+  Expr k;

Review comment:
   Should we use `Optional` here and it only holds value when 
input `k` is a constant?





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