[GitHub] [incubator-tvm] kevinthesun commented on issue #4969: [RFC] Enhance TensorFlow Frontend Control Flow Support

[GitBox]

kevinthesun commented on issue #4969:
URL: https://github.com/apache/incubator-tvm/issues/4969#issuecomment-643081334


   This feature is now supported in TVM.



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] zhiics commented on pull request #5699: [Frontend][TensorFlow] Improve Control Flow and TensorArray

[GitBox]

zhiics commented on pull request #5699:
URL: https://github.com/apache/incubator-tvm/pull/5699#issuecomment-643080759


   Thanks @kevinthesun @yongwww @lixiaoquan 



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

[incubator-tvm] branch master updated (54bde85 -> ae119f8)

[zhic]

This is an automated email from the ASF dual-hosted git repository.

zhic pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-tvm.git.


from 54bde85  [TIR][REFACTOR][API-Change] Migrate tir/stmt.h to use 
constructor. (#5778)
 add ae119f8  [Frontend][TensorFlow] Improve Control Flow and TensorArray 
(#5699)

No new revisions were added by this update.

Summary of changes:
 python/tvm/relay/frontend/common.py|   8 +-
 python/tvm/relay/frontend/pytorch.py   |  18 +-
 python/tvm/relay/frontend/tensorflow.py| 814 -
 python/tvm/relay/prelude.py|   9 +-
 src/relay/op/tensor/transform.cc   |  26 +-
 .../frontend/tensorflow/test_control_flow.py   |  26 +-
 tests/python/frontend/tensorflow/test_forward.py   |  81 +-
 tests/python/relay/test_adt.py |   1 -
 8 files changed, 602 insertions(+), 381 deletions(-)

[GitHub] [incubator-tvm] kevinthesun edited a comment on issue #4118: [RFC] Dynamic Shape Support - Graph Dispatching

[GitBox]

kevinthesun edited a comment on issue #4118:
URL: https://github.com/apache/incubator-tvm/issues/4118#issuecomment-643078760


   @zhanghaohit It's still under investigation for different options, but it's 
more likely a static shape will fall into a bucket and call corresponding 
kernel.



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] kevinthesun commented on issue #4118: [RFC] Dynamic Shape Support - Graph Dispatching

[GitBox]

kevinthesun commented on issue #4118:
URL: https://github.com/apache/incubator-tvm/issues/4118#issuecomment-643078760


   @zhanghaohit It's still under investigation for different options, but it's 
more likely a static shape will fall into a bucket and calling corresponding 
kernel.



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

[incubator-tvm] branch git-clang-format created (now f1876c7)

[zhic]

This is an automated email from the ASF dual-hosted git repository.

zhic pushed a change to branch git-clang-format
in repository https://gitbox.apache.org/repos/asf/incubator-tvm.git.


  at f1876c7  [DOC][FIX] Fix some typos in git-clang-format.sh

This branch includes the following new commits:

 new f1876c7  [DOC][FIX] Fix some typos in git-clang-format.sh

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

[incubator-tvm] 01/01: [DOC][FIX] Fix some typos in git-clang-format.sh

[zhic]

This is an automated email from the ASF dual-hosted git repository.

zhic pushed a commit to branch git-clang-format
in repository https://gitbox.apache.org/repos/asf/incubator-tvm.git

commit f1876c7b6e5c43e3fdc46ba81ffd554253652584
Author: Zhi <5145158+zhi...@users.noreply.github.com>
AuthorDate: Thu Jun 11 22:04:01 2020 -0700

[DOC][FIX] Fix some typos in git-clang-format.sh
---
 tests/lint/git-clang-format.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/lint/git-clang-format.sh b/tests/lint/git-clang-format.sh
index 5d2c4f3..b1ae1bc 100755
--- a/tests/lint/git-clang-format.sh
+++ b/tests/lint/git-clang-format.sh
@@ -25,7 +25,7 @@ if [[ "$#" -lt 1 ]]; then
 echo "Run clang-format on files that changed since "
 echo "Examples:"
 echo "- Compare last one commit: tests/lint/git-clang-format.sh HEAD~1"
-echo "- Compare against upstream/master: tests/lint/git-clang-format.sh 
upsstream/master"
+echo "- Compare against upstream/master: tests/lint/git-clang-format.sh 
upstream/master"
 echo "You can also add -i option to do inplace format"
 exit 1
 fi
@@ -48,7 +48,7 @@ CLANG_FORMAT=clang-format-10
 if [ -x "$(command -v clang-format-10)" ]; then
 CLANG_FORMAT=clang-format-10
 elif [ -x "$(command -v clang-format)" ]; then
-echo "clang-format might be different from clang-format-10, expect 
pontential difference."
+echo "clang-format might be different from clang-format-10, expect 
potential difference."
 CLANG_FORMAT=clang-format
 else
 echo "Cannot find clang-format-10"
@@ -70,6 +70,6 @@ echo "-clang-format log--"
 cat /tmp/$$.clang-format.txt
 echo ""
 if grep --quiet -E "diff" < /tmp/$$.clang-format.txt; then
-echo "clang-format lint error found. Consider run clang-format-10 on these 
files to fix them."
+echo "clang-format lint error found. Consider running clang-format-10 on 
these files to fix them."
 exit 1
 fi

[GitHub] [incubator-tvm] majiang31312 commented on issue #5686: [vulkan] Assertion in tir/transforms/lower_thread_allreduce.cc", line 157 TVMError: Check failed: v:

[GitBox]

majiang31312 commented on issue #5686:
URL: https://github.com/apache/incubator-tvm/issues/5686#issuecomment-643065326


   > @majiang31312 that sounds good, can you send a PR with your proposed fix?
   
   Thanks, I have create a PR for the fix.



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] majiang31312 opened a new pull request #5785: fix #5686: remove a overstrict assert in MakeAllreduce (#5686)

[GitBox]

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


   As #5686, MakeAllreduce impose a overstrict assert that the iteration 
argument must be a VarNode. But when extent of the iteration is 1, the 
iteration argument might be replace with a constant node. This fix allow this 
special case.
   @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:
us...@infra.apache.org

[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5770: [BYOC][runtime] Separate code and metadata for CSourceModule

[GitBox]

zhiics commented on a change in pull request #5770:
URL: https://github.com/apache/incubator-tvm/pull/5770#discussion_r439203922



##
File path: src/relay/backend/contrib/dnnl/codegen.cc
##
@@ -392,22 +381,46 @@ class DNNLModuleCodegen : public CSourceModuleCodegenBase 
{
 code_stream_ << "using namespace tvm::runtime::contrib;\n";
 code_stream_ << "\n";
 
+String func_symbol("all");

Review comment:
   I think it is okay that this is code a bit redundant to the one in 
c_codegen, because we are planning to remove this in the followup PRs and 
replace it with a json runtime.





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] kazum commented on pull request #5052: [TARGET] ONNX codegen

[GitBox]

kazum commented on pull request #5052:
URL: https://github.com/apache/incubator-tvm/pull/5052#issuecomment-643058073


   @srkreddy1238 @tqchen can you update your review status if the current 
version looks good to you?



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] yzhliu commented on pull request #5618: [Arith] Inequalities solver

[GitBox]

yzhliu commented on pull request #5618:
URL: https://github.com/apache/incubator-tvm/pull/5618#issuecomment-643049731


   Also cc @xqdan @junrushao1994 if you have time review.



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] tqchen commented on pull request #5784: [REFACTOR][API-Change] Migrate all Object construction to constructor.

[GitBox]

tqchen commented on pull request #5784:
URL: https://github.com/apache/incubator-tvm/pull/5784#issuecomment-643042290


   cc @yzhliu @ZihengJiang @junrushao1994 @icemelon9 @mbaret @merrymercy 
@Hzfengsy @zhiics @jroesch



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] tqchen opened a new pull request #5784: [REFACTOR][API-Change] Migrate all Object construction to constructor.

[GitBox]

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


   This PR migrates all the remaining object constructions to the new 
constructor style
   that is consistent with the rest of the codebase and changes the affected 
files accordingly.
   
   Other changes:
   
   - ThreadScope::make -> ThreadScope::Create
   - StorageScope::make -> StorageScope::Create
   



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] tom-gall commented on a change in pull request #5655: Add MicroTVM tutorial using the STM32F746 discovery board

[GitBox]

tom-gall commented on a change in pull request #5655:
URL: https://github.com/apache/incubator-tvm/pull/5655#discussion_r439184763



##
File path: tutorials/micro/micro_tflite.py
##
@@ -0,0 +1,219 @@
+# 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.
+"""
+.. _tutorial-micro-tflite:
+
+Micro TVM with TFLite Models
+
+**Author**: `Tom Gall `_
+
+This tutorial is an introduction to working with MicroTVM and TFLite models 
with Relay.
+"""
+##
+# Setup
+# -
+#
+# To get started, TFLite package needs to be installed as prerequisite.
+# 
+# install tflite
+# .. code-block:: bash
+#
+#   pip install tflite=2.1.0 --user
+#
+# or you could generate TFLite package yourself. The steps are the following:
+#
+#   Get the flatc compiler.
+#   Please refer to https://github.com/google/flatbuffers for details
+#   and make sure it is properly installed.
+#
+# .. code-block:: bash
+#
+#   flatc --version
+#
+# Get the TFLite schema.
+#
+# .. code-block:: bash
+#
+#   wget 
https://raw.githubusercontent.com/tensorflow/tensorflow/r1.13/tensorflow/lite/schema/schema.fbs
+#
+# Generate TFLite package.
+#
+# .. code-block:: bash
+#
+#   flatc --python schema.fbs
+#
+# Add current folder (which contains generated tflite module) to PYTHONPATH.
+#
+# .. code-block:: bash
+#
+#   export PYTHONPATH=${PYTHONPATH:+$PYTHONPATH:}$(pwd)
+#
+# To validate that the TFLite package was installed successfully, ``python -c 
"import tflite"``
+#
+# CMSIS needs to be downloaded and the CMSIS_ST_PATH environment variable setup
+# This tutorial only supports the STM32F7xx series of boards.
+# Download from : https://www.st.com/en/embedded-software/stm32cubef7.html
+# After you've expanded the zip file
+#
+# .. code-block:: bash
+#
+# export CMSIS_ST_PATH=/path/to/STM32Cube_FW_F7_V1.16.0/Drivers/CMSIS
+#
+# Next we need to download a pretrained TFLite model. When working with 
microcontrollers
+# you need to be mindful these are highly resource constrained devices as such 
standard 
+# models like Mobilenet may not fit into their modest memory. 
+#
+# For this tutorial, we'll make use of one of the TF Micro example models.
+# 
+# If you wish to replicate the training steps see:
+# 
https://github.com/tensorflow/tensorflow/tree/master/tensorflow/lite/micro/examples/hello_world/train
+#
+# .. code-block:: bash
+#
+# if you download the example pretrained model from
+#   wget 
https://storage.googleapis.com/download.tensorflow.org/models/tflite/micro/hello_world_2020_04_13.zip
+#   unzip hello_world_2020_04_13.zip
+#   this will fail due to an unimplemented opcode (114)
+#   I've saved an older version of the pre-trailed model and made it available 
on linaro.org
+
+##
+# Python imports for tvm, numpy etc
+# --
+import os
+import numpy as np
+import tvm
+import tvm.micro as micro
+import requests
+
+from tvm.contrib import graph_runtime, util
+from tvm import relay
+
+
+##
+# Load the pretrained TFLite model from a file in your current 
+# directory into a buffer
+model_url = 'https://people.linaro.org/~tom.gall/sine_model.tflite'

Review comment:
   I've gone ahead with this version and moved to the use of 
download_testdata. I've included the very simple model as part of this pull 
request, placing it in the same directory as the tutorial. Is this the right 
place to put 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.

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

[GitHub] [incubator-tvm] lixiaoquan commented on pull request #5780: [Fix] Fix recursive let for well formed check

[GitBox]

lixiaoquan commented on pull request #5780:
URL: https://github.com/apache/incubator-tvm/pull/5780#issuecomment-643039312


   Thanks for fixing this, I found other passes also have similar issue. This 
is VarVisitor used in FreeVars().
   
   
   
   ```
   diff --git a/src/relay/analysis/util.cc b/src/relay/analysis/util.cc
   index 0885a35df..633c3b065 100644
   --- a/src/relay/analysis/util.cc
   +++ b/src/relay/analysis/util.cc
   @@ -214,9 +214,13 @@ class VarVisitor : protected ExprVisitor, protected 
PatternVisitor {
  }

  void VisitExpr_(const LetNode* op) final {
   -MarkBounded(op->var);
   -VisitExpr(op->value);
   -VisitExpr(op->body);
   +Expr let = GetRef(op);
   +while (auto let_node = let.as()) {
   +  MarkBounded(let_node->var);
   +  VisitExpr(let_node->value);
   +  let = let_node->body;
   +}
   +VisitExpr(let);
  }

  void VisitPattern(const Pattern& p) final { 
PatternVisitor::VisitPattern(p); }
   ```
   
   I also run into stack overflow in DeDup() and ConstantFold(), but I'm not 
sure whether they are similar issue.



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] tqchen commented on issue #5686: [vulkan] Assertion in tir/transforms/lower_thread_allreduce.cc", line 157 TVMError: Check failed: v:

[GitBox]

tqchen commented on issue #5686:
URL: https://github.com/apache/incubator-tvm/issues/5686#issuecomment-643033427


   @majiang31312 that sounds good, can you send a PR with your proposed fix?



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] zhanghaohit commented on issue #4118: [RFC] Dynamic Shape Support - Graph Dispatching

[GitBox]

zhanghaohit commented on issue #4118:
URL: https://github.com/apache/incubator-tvm/issues/4118#issuecomment-643030579


   > > ![dg1 
(1)](https://user-images.githubusercontent.com/15520525/66729153-672d2a00-edfe-11e9-99f9-56b059e41b3b.png)
   > 
   > I'm still curious what will happen if we have conv2d(5, 3, 224, 224)? 
We'll use conv2d(8, 3, 224, 224)? Do we need to do some padding to use the 
kernel conv2d(8, 3, 224, 224)?
   > 
   > Thanks @kevinthesun for the clarification.
   
   Hi @kevinthesun . For this question, do we plan to do padding or resize? 
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:
us...@infra.apache.org

[GitHub] [incubator-tvm] majiang31312 edited a comment on issue #5686: [vulkan] Assertion in tir/transforms/lower_thread_allreduce.cc", line 157 TVMError: Check failed: v:

[GitBox]

majiang31312 edited a comment on issue #5686:
URL: https://github.com/apache/incubator-tvm/issues/5686#issuecomment-643029417


   > @majiang31312 can you please followup with another pr :)?
   
 Hi @tqchen, do you mean #5726? 
 I'm strongly agree with @t-vi  that this problem(5686) should be fixed 
separately. This problem is not related to any specific backend. In my opinion, 
it's just a misunderstanding in MakeAllreduce that impose a overstrict assert.
   



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] majiang31312 commented on issue #5686: [vulkan] Assertion in tir/transforms/lower_thread_allreduce.cc", line 157 TVMError: Check failed: v:

[GitBox]

majiang31312 commented on issue #5686:
URL: https://github.com/apache/incubator-tvm/issues/5686#issuecomment-643029417


   > @majiang31312 can you please followup with another pr :)?
 Hi @tqchen, do you mean #5726? 
 I'm strongly agree with @t-vi  that this problem(5686) should be fixed 
separately. This problem is not related to any specific backend. In my opinion, 
it's just a misunderstanding in MakeAllreduce that impose a overstrict assert.
   



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] kevinthesun commented on a change in pull request #5699: [Frontend][TensorFlow] Improve Control Flow and TensorArray

[GitBox]

kevinthesun commented on a change in pull request #5699:
URL: https://github.com/apache/incubator-tvm/pull/5699#discussion_r439144860



##
File path: python/tvm/relay/frontend/tensorflow.py
##
@@ -1177,18 +1137,26 @@ def _impl(inputs, attr, params, mod):
 except (IndexError, KeyError, AttributeError):
 # Handle symbolic begin
 try:
-begin = _infer_value(inputs[1], params).asnumpy().tolist()
+begin = _infer_value(inputs[1], params, mod).asnumpy().tolist()
 except Exception:
 begin = inputs[1]
 try:
 size = _get_list_param(params, inputs[2])
 except (IndexError, KeyError, AttributeError):
 # Handle symbolic size
 try:
-size = _infer_value(inputs[2], params).asnumpy().tolist()
+size = _infer_value(inputs[2], params, mod).asnumpy().tolist()
 except Exception:
 size = inputs[2]
-return _op.strided_slice(inputs[0], begin=begin, end=size, 
slice_mode="size")
+
+# Align begin and strides for dynamic shape.
+data_dim = len(_infer_shape(inputs[0], mod))
+strides = [1] * data_dim
+if not isinstance(begin, (_expr.Call, _expr.Var)):
+for _ in range(len(begin), data_dim):
+begin.append(0)

Review comment:
   Added.





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] hlu1 opened a new pull request #5783: [Topi] pass-by-value -> pass-by-const-reference

[GitBox]

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


   Mostly to unify the API of `schedule_injective`, `schedule_extern`, and 
`default_schedule` by moving to pass-by-const-reference.



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] yongwww commented on a change in pull request #5699: [Frontend][TensorFlow] Improve Control Flow and TensorArray

[GitBox]

yongwww commented on a change in pull request #5699:
URL: https://github.com/apache/incubator-tvm/pull/5699#discussion_r439130129



##
File path: python/tvm/relay/frontend/tensorflow.py
##
@@ -1177,18 +1137,26 @@ def _impl(inputs, attr, params, mod):
 except (IndexError, KeyError, AttributeError):
 # Handle symbolic begin
 try:
-begin = _infer_value(inputs[1], params).asnumpy().tolist()
+begin = _infer_value(inputs[1], params, mod).asnumpy().tolist()
 except Exception:
 begin = inputs[1]
 try:
 size = _get_list_param(params, inputs[2])
 except (IndexError, KeyError, AttributeError):
 # Handle symbolic size
 try:
-size = _infer_value(inputs[2], params).asnumpy().tolist()
+size = _infer_value(inputs[2], params, mod).asnumpy().tolist()
 except Exception:
 size = inputs[2]
-return _op.strided_slice(inputs[0], begin=begin, end=size, 
slice_mode="size")
+
+# Align begin and strides for dynamic shape.
+data_dim = len(_infer_shape(inputs[0], mod))
+strides = [1] * data_dim
+if not isinstance(begin, (_expr.Call, _expr.Var)):
+for _ in range(len(begin), data_dim):
+begin.append(0)

Review comment:
   apply similar append to size? `size.append(-1)`





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] antinucleon edited a comment on pull request #5782: [topi] fix strategy for sparse dense cuda

[GitBox]

antinucleon edited a comment on pull request #5782:
URL: https://github.com/apache/incubator-tvm/pull/5782#issuecomment-642984777


   @vinx13 @ceruleangu 
   
   I fixed the strategy so it is able to lower now. However, the schedule is 
not working with the current master. Could you fix it?
   
   To verify it, please enable CUDA test at:
   
https://github.com/apache/incubator-tvm/blob/master/tests/python/relay/test_sparse_dense_convert.py#L50
   
   Maybe you can take away this PR and send new PR to address these issues. 
Once it is fixed I will close this 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:
us...@infra.apache.org

[GitHub] [incubator-tvm] junrushao1994 commented on a change in pull request #5782: [topi] fix strategy for sparse dense cuda

[GitBox]

junrushao1994 commented on a change in pull request #5782:
URL: https://github.com/apache/incubator-tvm/pull/5782#discussion_r439130050



##
File path: python/tvm/relay/op/strategy/generic.py
##
@@ -599,12 +599,30 @@ def batch_matmul_strategy(attrs, inputs, out_type, 
target):
 name="batch_matmul.generic")
 return strategy
 
-# sparse_dense
-@generic_func
-def schedule_sparse_dense(attrs, outs, target):
-"""schedule sparse_dense"""
-with target:
-return topi.generic.schedule_sparse_dense(outs)
+# sparse dense
+def wrap_compute_sparse_dense(topi_compute):
+"""wrap sparse dense topi compute"""
+def _compute_sparse_dense(attrs, inputs, out_type):
+return [topi_compute(inputs[0], inputs[1], inputs[2], inputs[3])]
+return _compute_sparse_dense
+
+@override_native_generic_func("sparse_dense_strategy")
+def sparse_dense_strategy(attrs, inputs, out_type, target):
+"""sparse dense generic strategy"""
+logger.warning("sparse dense is not optimized for this platform.")
+strategy = _op.OpStrategy()
+
strategy.add_implementation(wrap_compute_sparse_dense(topi.nn.sparse_dense),
+
wrap_topi_schedule(topi.generic.schedule_sparse_dense),
+name="sparse_dense.generic")
+return strategy
+
+
+# # sparse_dense

Review comment:
   remove those lines?





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] antinucleon opened a new pull request #5782: [topi] fix strategy for sparse dense cuda

[GitBox]

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


   



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] masahi edited a comment on pull request #5755: Edit onnx parser to infer values in post order

[GitBox]

masahi edited a comment on pull request #5755:
URL: https://github.com/apache/incubator-tvm/pull/5755#issuecomment-642983173


   ok I now understand why it makes importing some model slower. Previously 
there are fewer but heavy `infer_value` calls, now there are many but they are 
all single node compile, so LLVM overhead dominates.
   
   I wish there were a "fast-path" compile option for llvm for use case like 
this. Turning on all optimization to compile a single node and get a compile 
time constant is clearly overkill. At least we can lower the opt level (always 
3 in our backend) for const eval mode @tqchen 
   
   Maybe we can add some option here to tell llvm that we want a fast path 
compile 
   
https://github.com/apache/incubator-tvm/blob/master/python/tvm/relay/frontend/common.py#L509
 



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] masahi commented on pull request #5755: Edit onnx parser to infer values in post order

[GitBox]

masahi commented on pull request #5755:
URL: https://github.com/apache/incubator-tvm/pull/5755#issuecomment-642983173


   ok I now understand why it makes importing some model slower. Previously 
there are fewer but heavy `infer_value` calls, now there are many but they are 
all single node compile, so LLVM overhead dominates.
   
   I wish there were a "fast-path" compile option for llvm for use case like 
this. Turning on all optimization to compile a single node and get a compile 
time constant is clearly overkill. At least we can lower the opt level (always 
3 in our backend) for const eval mode @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:
us...@infra.apache.org

[GitHub] [incubator-tvm] tqchen merged pull request #5778: [TIR][REFACTOR][API-Change] Migrate tir/stmt.h to use constructor.

[GitBox]

tqchen merged pull request #5778:
URL: https://github.com/apache/incubator-tvm/pull/5778


   



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

[incubator-tvm] branch master updated (eafb2aa -> 54bde85)

[tqchen]

This is an automated email from the ASF dual-hosted git repository.

tqchen pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-tvm.git.


from eafb2aa  [TIR][REFACTOR][API-Change] Migrate the tvm/tir/expr.h to 
construct style. (#5773)
 add 54bde85  [TIR][REFACTOR][API-Change] Migrate tir/stmt.h to use 
constructor. (#5778)

No new revisions were added by this update.

Summary of changes:
 include/tvm/tir/stmt.h | 151 -
 include/tvm/tir/stmt_functor.h |   2 +-
 src/arith/ir_mutator_with_analyzer.cc  |   2 +-
 src/target/llvm/codegen_cpu.cc |   3 +-
 src/te/operation/compute_op.cc |  14 +-
 src/te/operation/cross_thread_reduction.cc |  30 +-
 src/te/operation/extern_op.cc  |  10 +-
 src/te/operation/hybrid_op.cc  |  29 +-
 src/te/operation/op_util.cc|  33 +-
 src/te/operation/scan_op.cc|   8 +-
 src/te/operation/tensor_compute_op.cc  |   6 +-
 src/te/operation/tensorize.cc  |   6 +-
 src/te/schedule/operation_inline.cc|   2 +-
 src/te/schedule/schedule_dataflow_rewrite.cc   |  14 +-
 src/te/schedule/schedule_ops.cc|  29 +-
 .../schedule_postproc_rewrite_for_tensor_core.cc   |  37 +-
 src/te/schedule/schedule_postproc_to_primfunc.cc   |   8 +-
 src/tir/ir/buffer.cc   |   7 +-
 src/tir/ir/expr.cc |  22 +
 src/tir/ir/stmt.cc | 609 +++--
 src/tir/ir/stmt_functor.cc |   3 +-
 src/tir/pass/hoist_if_then_else.cc |  11 +-
 src/tir/transforms/arg_binder.cc   |  35 +-
 src/tir/transforms/bound_checker.cc|   8 +-
 src/tir/transforms/combine_context_call.cc |   2 +-
 src/tir/transforms/coproc_sync.cc  |  22 +-
 src/tir/transforms/decorate_device_scope.cc|   2 +-
 src/tir/transforms/inject_double_buffer.cc |  23 +-
 src/tir/transforms/inject_virtual_thread.cc|  17 +-
 src/tir/transforms/ir_util.cc  |  13 +-
 src/tir/transforms/ir_util.h   |   3 +-
 src/tir/transforms/lift_attr_scope.cc  |  16 +-
 src/tir/transforms/loop_partition.cc   |   8 +-
 src/tir/transforms/lower_custom_datatypes.cc   |   4 +-
 .../transforms/lower_device_storage_access_info.cc |   2 +-
 src/tir/transforms/lower_thread_allreduce.cc   |  58 +-
 src/tir/transforms/lower_tvm_builtin.cc|  35 +-
 src/tir/transforms/lower_warp_memory.cc|   9 +-
 src/tir/transforms/make_packed_api.cc  |  43 +-
 src/tir/transforms/narrow_datatype.cc  |   8 +-
 src/tir/transforms/remap_thread_axis.cc|   2 +-
 src/tir/transforms/remove_no_op.cc |  16 +-
 src/tir/transforms/simplify.cc |   2 +-
 src/tir/transforms/split_host_device.cc|   6 +-
 src/tir/transforms/storage_flatten.cc  |  32 +-
 src/tir/transforms/storage_rewrite.cc  |  35 +-
 src/tir/transforms/tensorcore_infer_fragment.cc|   6 +-
 src/tir/transforms/thread_storage_sync.cc  |  21 +-
 src/tir/transforms/unroll_loop.cc  |   6 +-
 src/tir/transforms/vectorize_loop.cc   |  18 +-
 tests/cpp/ir_functor_test.cc   |  26 +-
 topi/include/topi/detail/extern.h  |   2 +-
 52 files changed, 802 insertions(+), 714 deletions(-)

[GitHub] [incubator-tvm] zhiics commented on pull request #5699: [Frontend][TensorFlow] Improve Control Flow and TensorArray

[GitBox]

zhiics commented on pull request #5699:
URL: https://github.com/apache/incubator-tvm/pull/5699#issuecomment-642977258


   @lixiaoquan @yongwww please take another look when you have time.



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] jwfromm commented on pull request #5781: [Relay][Frontend][ONNX] Add storage_order ignore in pooling layer.

[GitBox]

jwfromm commented on pull request #5781:
URL: https://github.com/apache/incubator-tvm/pull/5781#issuecomment-642975407


   @masahi can you take a quick look at this 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:
us...@infra.apache.org

[GitHub] [incubator-tvm] kevinthesun commented on pull request #5699: [Frontend][TensorFlow] Improve Control Flow and TensorArray

[GitBox]

kevinthesun commented on pull request #5699:
URL: https://github.com/apache/incubator-tvm/pull/5699#issuecomment-642969988


   @zhiics @yongwww @lixiaoquan Comments addressed. PTAL.



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 pull request #5780: [Fix] Fix recursive let for well formed check

[GitBox]

icemelon9 commented on pull request #5780:
URL: https://github.com/apache/incubator-tvm/pull/5780#issuecomment-642957939


   @lixiaoquan Could you check if the pr fixes your issue?



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 opened a new pull request #5780: [Fix] Fix recursive let for well formed check

[GitBox]

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


   Thanks for contributing to TVM!   Please refer to guideline 
https://tvm.apache.org/docs/contribute/ for useful information and tips. After 
the pull request is submitted, please request code reviews from 
[Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers)
 by @ them in the pull request thread.
   
   cc @lixiaoquan @jroesch 



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] tqchen commented on issue #5686: [vulkan] Assertion in tir/transforms/lower_thread_allreduce.cc", line 157 TVMError: Check failed: v:

[GitBox]

tqchen commented on issue #5686:
URL: https://github.com/apache/incubator-tvm/issues/5686#issuecomment-642932336


   @majiang31312 can you please followup with another 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:
us...@infra.apache.org

[GitHub] [incubator-tvm] t-vi commented on issue #5686: [vulkan] Assertion in tir/transforms/lower_thread_allreduce.cc", line 157 TVMError: Check failed: v:

[GitBox]

t-vi commented on issue #5686:
URL: https://github.com/apache/incubator-tvm/issues/5686#issuecomment-642928736


   I do think there is a bug still to be fixed even if it is not currently 
triggered by the schedules, i.e.
   @majiang31312 's analysis looks very plausible and I can reproduce the 
assertion with the demo.



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] t-vi commented on pull request #5779: Improve separation of PyTorch dtypes and TVM dtypes in relay PyTorch frontend

[GitBox]

t-vi commented on pull request #5779:
URL: https://github.com/apache/incubator-tvm/pull/5779#issuecomment-642923575


   @masahi 
   
   So what I did here in addition to what is seen was to locally assert that no 
`float` is passed to const/var creation as dtype. This is related to the forum 
discussion here: 
https://discuss.tvm.ai/t/discuss-the-meaning-of-float-in-relay/6949/
   
   The next step will be to apply the data type of input tensors on scalar 
inputs, but I'll keep that for a separate PR.
   In addition to my primary use-case - allowing doubles to facilitate testing 
of conversions - this likely helps with mixed precision etc.



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] t-vi opened a new pull request #5779: Improve separation of PyTorch dtypes and TVM dtypes in relay PyTorch frontend

[GitBox]

t-vi opened a new pull request #5779:
URL: https://github.com/apache/incubator-tvm/pull/5779


   Previously, we sometimes used type strings taken from PyTorch (e.g. float) 
in TVM. Now we aim to convert PyTorch dtypes early and only work with TVM 
dtypes afterwards.
   
   Also fix arange/linspace type semantics.
   
   A follow-up will then further improve type support (at which point we will 
be able to convert e.g. double models).
   



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] tqchen closed issue #5686: [vulkan] Assertion in tir/transforms/lower_thread_allreduce.cc", line 157 TVMError: Check failed: v:

[GitBox]

tqchen closed issue #5686:
URL: https://github.com/apache/incubator-tvm/issues/5686


   



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] tqchen commented on issue #5686: [vulkan] Assertion in tir/transforms/lower_thread_allreduce.cc", line 157 TVMError: Check failed: v:

[GitBox]

tqchen commented on issue #5686:
URL: https://github.com/apache/incubator-tvm/issues/5686#issuecomment-642915735


   https://github.com/apache/incubator-tvm/pull/5726



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] comaniac commented on pull request #5768: [Relay] Fix Calibration Pass to Support Modules with Multiple Functions

[GitBox]

comaniac commented on pull request #5768:
URL: https://github.com/apache/incubator-tvm/pull/5768#issuecomment-642914327


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

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

[GitHub] [incubator-tvm] tqchen commented on pull request #5666: [TIR][Bugfix] Improved massive build times caused by tir.floormod and tir.floordiv. Fixed Topi testcase.

[GitBox]

tqchen commented on pull request #5666:
URL: https://github.com/apache/incubator-tvm/pull/5666#issuecomment-642899544


   cc @dprankratz can we followup 



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] tqchen commented on a change in pull request #5777: [cmake] update vulkan build rule

[GitBox]

tqchen commented on a change in pull request #5777:
URL: https://github.com/apache/incubator-tvm/pull/5777#discussion_r439018309



##
File path: cmake/util/FindVulkan.cmake
##
@@ -60,7 +60,7 @@ macro(find_vulkan use_vulkan)
# additional libraries
 
   if(Vulkan_FOUND)
-get_filename_component(VULKAN_LIBRARY_PATH ${Vulkan_LIBRARY} DIRECTORY)
+set(VULKAN_LIBRARY_PATH ${__vulkan_sdk}/lib)

Review comment:
   We cannot use `__vulkan_sdk` here directly, because it may not be 
available in the alternative case (Cmake's official find vulkan). And ` 
USE_VULKAN ON`





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] t-vi commented on pull request #5756: make dtype="float" mean float32 for relay.expr.const

[GitBox]

t-vi commented on pull request #5756:
URL: https://github.com/apache/incubator-tvm/pull/5756#issuecomment-642874651


   I created https://discuss.tvm.ai/t/the-meaning-of-float-in-relay/6949 with 
an attempt to summarize things.



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] tqchen commented on pull request #5778: [TIR][REFACTOR][API-Change] Migrate tir/stmt.h to use constructor.

[GitBox]

tqchen commented on pull request #5778:
URL: https://github.com/apache/incubator-tvm/pull/5778#issuecomment-642861793


   cc @yzhliu @ZihengJiang @junrushao1994 @icemelon9 @mbaret @merrymercy 
@Hzfengsy @zhiics @jroesch



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] tqchen opened a new pull request #5778: [TIR][REFACTOR][API-Change] Migrate tir/stmt.h to use constructor.

[GitBox]

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


   This PR migrate tvm/tir/stmt.h to the new constructor style that is
   consistent with the rest of the codebase and changes the affected files 
accordingly.
   



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

[incubator-tvm] branch master updated (0abcad1 -> eafb2aa)

[tqchen]

This is an automated email from the ASF dual-hosted git repository.

tqchen pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-tvm.git.


from 0abcad1  Make batch matrix multiplication on GPU tunable (#5752)
 add eafb2aa  [TIR][REFACTOR][API-Change] Migrate the tvm/tir/expr.h to 
construct style. (#5773)

No new revisions were added by this update.

Summary of changes:
 docs/dev/relay_add_op.rst  |2 +-
 docs/dev/relay_add_pass.rst|2 +-
 docs/dev/relay_pass_infra.rst  |2 +-
 include/tvm/relay/type.h   |3 +-
 include/tvm/runtime/container.h|4 +-
 include/tvm/runtime/object.h   |2 +
 include/tvm/te/tensor_intrin.h |   34 +-
 include/tvm/tir/expr.h |  383 +--
 include/tvm/tir/op.h   |8 +-
 include/tvm/tir/stmt.h |2 +-
 include/tvm/tir/var.h  |   51 +-
 src/arith/canonical_simplify.cc|   32 +-
 src/arith/const_fold.h |   40 +-
 src/arith/detect_linear_equation.cc|4 +-
 src/arith/int_set.cc   |   97 +-
 src/arith/ir_mutator_with_analyzer.cc  |   14 +-
 src/arith/pattern_match.h  |  103 +-
 src/arith/rewrite_simplify.cc  |   36 +-
 src/autotvm/touch_extractor.cc |8 +-
 src/ir/expr.cc |2 +-
 src/relay/analysis/type_solver.cc  |4 +-
 src/relay/analysis/util.cc |2 +-
 src/relay/ir/dataflow_matcher.cc   |2 +-
 src/relay/ir/expr.cc   |2 +-
 src/relay/op/algorithm/topk.cc |2 +-
 src/relay/op/nn/upsampling.cc  |   10 +-
 src/relay/op/tensor/transform.cc   |   38 +-
 src/relay/op/tensor/transform.h|6 +-
 src/relay/op/type_relations.cc |4 +-
 src/target/intrin_rule.h   |2 +-
 src/target/llvm/codegen_arm.cc |   13 +-
 src/target/llvm/codegen_cpu.cc |5 +-
 src/target/llvm/codegen_x86_64.cc  |   17 +-
 src/target/llvm/intrin_rule_llvm.cc|   25 +-
 src/target/llvm/intrin_rule_llvm.h |4 +-
 src/target/llvm/intrin_rule_nvptx.cc   |2 +-
 src/target/llvm/intrin_rule_rocm.cc|   18 +-
 src/target/source/intrin_rule_cuda.cc  |2 +-
 src/target/source/intrin_rule_opencl.cc|2 +-
 src/target/spirv/intrin_rule_spirv.cc  |2 +-
 src/te/autodiff/ad_util.cc |7 +-
 src/te/autodiff/adjoint.cc |2 +-
 src/te/autodiff/jacobian.cc|   60 +-
 src/te/operation/compute_op.cc |   15 +-
 src/te/operation/cross_thread_reduction.cc |   14 +-
 src/te/operation/extern_op.cc  |2 +-
 src/te/operation/hybrid_op.cc  |6 +-
 src/te/operation/scan_op.cc|   10 +-
 src/te/operation/tensor_compute_op.cc  |6 +-
 src/te/operation/tensorize.cc  |8 +-
 src/te/schedule/operation_inline.cc|2 +-
 src/te/schedule/schedule_dataflow_rewrite.cc   |   26 +-
 src/te/schedule/schedule_lang.cc   |   14 +-
 src/te/schedule/schedule_ops.cc|3 +-
 .../schedule_postproc_rewrite_for_tensor_core.cc   |   72 +-
 src/te/tensor.cc   |   23 +-
 src/tir/ir/buffer.cc   |   17 +-
 src/tir/ir/data_layout.cc  |   14 +-
 src/tir/ir/expr.cc | 1120 +++-
 src/tir/ir/expr_functor.cc |   75 +-
 src/tir/ir/op.cc   |  184 ++--
 src/tir/ir/stmt.cc |2 +-
 src/tir/ir/stmt_functor.cc |2 +-
 src/tir/transforms/arg_binder.cc   |   42 +-
 src/tir/transforms/bound_checker.cc|   22 +-
 src/tir/transforms/coproc_sync.cc  |   21 +-
 src/tir/transforms/inject_copy_intrin.cc   |2 +-
 src/tir/transforms/inject_double_buffer.cc |9 +-
 src/tir/transforms/inject_virtual_thread.cc|7 +-
 src/tir/transforms/ir_util.cc  |5 +-
 src/tir/transforms/ir_util.h   |   20 +-
 src/tir/transforms/loop_partition.cc   |   16 +-
 src/tir/transforms/lower_custom_datatypes.cc   |2 +-
 

[GitHub] [incubator-tvm] antinucleon opened a new pull request #5777: [cmake] update vulkan build rule

[GitBox]

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


   Original rule will fail if there is a libvulkan.so existing in the system 
lib directory. 



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] kevinthesun commented on a change in pull request #5699: [Frontend][TensorFlow] Improve Control Flow and TensorArray

[GitBox]

kevinthesun commented on a change in pull request #5699:
URL: https://github.com/apache/incubator-tvm/pull/5699#discussion_r438983100



##
File path: python/tvm/relay/frontend/tensorflow.py
##
@@ -2395,29 +2410,40 @@ def _get_abs_layer_name(node):
 # 1.x.
 _control_flow_nodes = ['Merge', 'Switch', 'NextIteration', 'Exit', 'Enter', 
'LoopCond']
 
-# A map to record tensor array with fixed rank shape
-_static_tensor_array_map = {}
-
-class RewriteSubgraph(ExprMutator):
-"""
-A helper class to rewrite expr in while loop function to variable
-
-Parameters
---
-rewrite_map : Dict[expr, expr]
-A dictionay contains a set of expr to var mapping.
-"""
-def __init__(self, rewrite_map):
-ExprMutator.__init__(self)
-self.rewrite_map = rewrite_map
-
-def visit(self, expr):
-if expr in self.rewrite_map:
-return self.rewrite_map[expr]
-return super().visit(expr)
+# A map to record tensor array write ops and input ta/tensor indices
+# Value is (index of tensor array, index of written node)
+_tensor_array_write_ops = {
+"TensorArrayWrite"   : (3, 2),
+"TensorArrayScatter" : (0, 2),
+"TensorArraySplit"   : (0, 1),
+}
 
-def rewrite_subgraph(expr, rewrites):
-return RewriteSubgraph(rewrites).visit(expr)
+def is_tensor_array_constuctor(tf_node):
+"""Check whether is tensor array constructor node."""
+is_ta = False
+ta_start = "TensorArrayV"
+if tf_node.op.startswith(ta_start):
+try:
+int(tf_node.op[len(ta_start)])

Review comment:
   Will use ```isnumeric``` instead

##
File path: python/tvm/relay/frontend/tensorflow.py
##
@@ -2395,29 +2410,40 @@ def _get_abs_layer_name(node):
 # 1.x.
 _control_flow_nodes = ['Merge', 'Switch', 'NextIteration', 'Exit', 'Enter', 
'LoopCond']
 
-# A map to record tensor array with fixed rank shape
-_static_tensor_array_map = {}
-
-class RewriteSubgraph(ExprMutator):
-"""
-A helper class to rewrite expr in while loop function to variable
-
-Parameters
---
-rewrite_map : Dict[expr, expr]
-A dictionay contains a set of expr to var mapping.
-"""
-def __init__(self, rewrite_map):
-ExprMutator.__init__(self)
-self.rewrite_map = rewrite_map
-
-def visit(self, expr):
-if expr in self.rewrite_map:
-return self.rewrite_map[expr]
-return super().visit(expr)
+# A map to record tensor array write ops and input ta/tensor indices
+# Value is (index of tensor array, index of written node)
+_tensor_array_write_ops = {
+"TensorArrayWrite"   : (3, 2),
+"TensorArrayScatter" : (0, 2),
+"TensorArraySplit"   : (0, 1),
+}
 
-def rewrite_subgraph(expr, rewrites):
-return RewriteSubgraph(rewrites).visit(expr)
+def is_tensor_array_constuctor(tf_node):
+"""Check whether is tensor array constructor node."""
+is_ta = False
+ta_start = "TensorArrayV"
+if tf_node.op.startswith(ta_start):
+try:
+int(tf_node.op[len(ta_start)])

Review comment:
   Will use ```isnumeric``` instead





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] kevinthesun commented on a change in pull request #5699: [Frontend][TensorFlow] Improve Control Flow and TensorArray

[GitBox]

kevinthesun commented on a change in pull request #5699:
URL: https://github.com/apache/incubator-tvm/pull/5699#discussion_r438983100



##
File path: python/tvm/relay/frontend/tensorflow.py
##
@@ -2395,29 +2410,40 @@ def _get_abs_layer_name(node):
 # 1.x.
 _control_flow_nodes = ['Merge', 'Switch', 'NextIteration', 'Exit', 'Enter', 
'LoopCond']
 
-# A map to record tensor array with fixed rank shape
-_static_tensor_array_map = {}
-
-class RewriteSubgraph(ExprMutator):
-"""
-A helper class to rewrite expr in while loop function to variable
-
-Parameters
---
-rewrite_map : Dict[expr, expr]
-A dictionay contains a set of expr to var mapping.
-"""
-def __init__(self, rewrite_map):
-ExprMutator.__init__(self)
-self.rewrite_map = rewrite_map
-
-def visit(self, expr):
-if expr in self.rewrite_map:
-return self.rewrite_map[expr]
-return super().visit(expr)
+# A map to record tensor array write ops and input ta/tensor indices
+# Value is (index of tensor array, index of written node)
+_tensor_array_write_ops = {
+"TensorArrayWrite"   : (3, 2),
+"TensorArrayScatter" : (0, 2),
+"TensorArraySplit"   : (0, 1),
+}
 
-def rewrite_subgraph(expr, rewrites):
-return RewriteSubgraph(rewrites).visit(expr)
+def is_tensor_array_constuctor(tf_node):
+"""Check whether is tensor array constructor node."""
+is_ta = False
+ta_start = "TensorArrayV"
+if tf_node.op.startswith(ta_start):
+try:
+int(tf_node.op[len(ta_start)])

Review comment:
   Will use ```isnumeric``` instead





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] kevinthesun commented on a change in pull request #5762: [TF] Support symbolic inputs of Fill

[GitBox]

kevinthesun commented on a change in pull request #5762:
URL: https://github.com/apache/incubator-tvm/pull/5762#discussion_r438948105



##
File path: python/tvm/relay/frontend/tensorflow.py
##
@@ -1352,12 +1352,17 @@ def _impl(inputs, attr, params, mod):
 # Output shape must be defined to avoid errors. If any axis is not, we 
must
 # try to compute its shape.
 if output_shape is None or -1 in output_shape:
-output_shape = _infer_value(inputs[0], 
params).asnumpy().reshape([-1]).tolist()
+try:
+output_shape = _infer_value(inputs[0], params, 
mod).asnumpy().reshape([-1]).tolist()
+except (IndexError, KeyError, AttributeError):

Review comment:
   Need a general exception for _infer_value





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] siju-samuel opened a new pull request #5776: [PYTORCH]aten::norm support added

[GitBox]

siju-samuel opened a new pull request #5776:
URL: https://github.com/apache/incubator-tvm/pull/5776


   Support added for the below ops.
   
   -- aten::norm
   -- aten::frobenius_norm
   
   https://github.com/apache/incubator-tvm/issues/5133#issuecomment-642632617
   
   @masahi Please help to review this PR. TIA
   
   cc @SJTU-yys 



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] giuseros commented on pull request #5754: [RFC] Improve quantized convolution performance for armv8 architectures

[GitBox]

giuseros commented on pull request #5754:
URL: https://github.com/apache/incubator-tvm/pull/5754#issuecomment-642801815


   Hi @FrozenGene ,
   I gave it another go, but switching legalization on the strategy seems very 
hard (since we would need the auto-tuner to pick the best data-type for us). 
   
   So for now, we have to content  with the `_alter_conv2d_layout` workaround 
and try to think a bit more on how we can infer the strategy during  
legalization



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] tqchen commented on pull request #5752: Make batch matrix multiplication on GPU tunable

[GitBox]

tqchen commented on pull request #5752:
URL: https://github.com/apache/incubator-tvm/pull/5752#issuecomment-642797574


   Thanks @t-vi  for the contribution, and @comaniac 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:
us...@infra.apache.org

[GitHub] [incubator-tvm] tqchen merged pull request #5752: Make batch matrix multiplication on GPU tunable

[GitBox]

tqchen merged pull request #5752:
URL: https://github.com/apache/incubator-tvm/pull/5752


   



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

[incubator-tvm] branch master updated (0d2b4bc -> 0abcad1)

[tqchen]

This is an automated email from the ASF dual-hosted git repository.

tqchen pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-tvm.git.


from 0d2b4bc  Add ShapePattern and DataTypePattern (#5760)
 add 0abcad1  Make batch matrix multiplication on GPU tunable (#5752)

No new revisions were added by this update.

Summary of changes:
 python/tvm/relay/op/strategy/cuda.py|  2 +-
 topi/python/topi/cuda/batch_matmul.py   | 80 -
 topi/tests/python/test_topi_batch_matmul.py |  2 +-
 3 files changed, 58 insertions(+), 26 deletions(-)

[GitHub] [incubator-tvm] tqchen edited a comment on pull request #5773: [TIR][REFACTOR][API-Change] Migrate the tvm/tir/expr.h to constructor

[GitBox]

tqchen edited a comment on pull request #5773:
URL: https://github.com/apache/incubator-tvm/pull/5773#issuecomment-642759334


   cc  @yzhliu @ZihengJiang @junrushao1994 @icemelon9 @mbaret @merrymercy 
@Hzfengsy @zhiics @jroesch 



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] tqchen edited a comment on pull request #5773: [TIR][REFACTOR][API-Change] Migrate the tvm/tir/expr.h to constructor

[GitBox]

tqchen edited a comment on pull request #5773:
URL: https://github.com/apache/incubator-tvm/pull/5773#issuecomment-642759334


   cc  @yzhliu @ZihengJiang @junrushao1994 @icemelon9 @mbaret @merrymercy 
@Hzfengsy @zhiics 



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] tqchen edited a comment on pull request #5773: [TIR][REFACTOR][API-Change] Migrate the tvm/tir/expr.h to constructor

[GitBox]

tqchen edited a comment on pull request #5773:
URL: https://github.com/apache/incubator-tvm/pull/5773#issuecomment-642759334


   cc  @yzhliu @ZihengJiang @junrushao1994 @icemelon9 @mbaret @merrymercy 



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] tqchen commented on pull request #5773: [TIR][REFACTOR][API-Change] Migrate the tvm/tir/expr.h to constructor

[GitBox]

tqchen commented on pull request #5773:
URL: https://github.com/apache/incubator-tvm/pull/5773#issuecomment-642759334


   cc  @yzhliu @ZihengJiang @junrushao1994 @icemelon9 @mbaret 



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] siju-samuel opened a new pull request #5775: [TENSORFLOW]Conv3d Transpose OP added

[GitBox]

siju-samuel opened a new pull request #5775:
URL: https://github.com/apache/incubator-tvm/pull/5775


   Conv3d transpose op support added to tensorflow frontend
   
   @masahi @FrozenGene please help to review this PR. TIA



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] mbaret commented on a change in pull request #5634: [CODEGEN][CONTRIB] CoreML codegen

[GitBox]

mbaret commented on a change in pull request #5634:
URL: https://github.com/apache/incubator-tvm/pull/5634#discussion_r433473770



##
File path: python/tvm/contrib/target/coreml.py
##
@@ -0,0 +1,226 @@
+# 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, unused-argument, import-outside-toplevel
+"""Utility to compile CoreML models"""
+
+import os
+import shutil
+
+import tvm._ffi
+from ...relay.expr_functor import ExprVisitor
+from .. import xcode, coreml_runtime
+
+def _convert_add(builder, name, inputs, outputs, args, attrs):
+builder.add_elementwise(

Review comment:
   Will this handle broadcast ADDs?





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] masahi merged pull request #5760: [PatternLang] Add ShapePattern and DataTypePattern

[GitBox]

masahi merged pull request #5760:
URL: https://github.com/apache/incubator-tvm/pull/5760


   



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

[incubator-tvm] branch master updated (672e6cd -> 0d2b4bc)

[masahi]

This is an automated email from the ASF dual-hosted git repository.

masahi pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-tvm.git.


from 672e6cd  Fix gelu in PyTorch frontend, tighten numerical checks (#5763)
 add 0d2b4bc  Add ShapePattern and DataTypePattern (#5760)

No new revisions were added by this update.

Summary of changes:
 include/tvm/relay/dataflow_pattern.h  |  58 +
 include/tvm/relay/dataflow_pattern_functor.h  |  10 ++-
 python/tvm/relay/dataflow_pattern/__init__.py | 120 --
 src/relay/ir/dataflow_matcher.cc  |  20 -
 src/relay/ir/dataflow_pattern.cc  |  40 +
 src/relay/ir/dataflow_pattern_functor.cc  |   7 ++
 src/relay/ir/indexed_graph.cc |  13 ++-
 tests/python/relay/test_dataflow_pattern.py   |  92 
 8 files changed, 350 insertions(+), 10 deletions(-)

[GitHub] [incubator-tvm] masahi commented on pull request #5760: [PatternLang] Add ShapePattern and DataTypePattern

[GitBox]

masahi commented on pull request #5760:
URL: https://github.com/apache/incubator-tvm/pull/5760#issuecomment-642693067


   Thanks @mbrookhart 



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] FrozenGene commented on pull request #5754: [RFC] Improve quantized convolution performance for armv8 architectures

[GitBox]

FrozenGene commented on pull request #5754:
URL: https://github.com/apache/incubator-tvm/pull/5754#issuecomment-642690898


   > Hi @FrozenGene ,
   > I agree that different strategies should be available to the auto-tuner. 
See if the solution proposed is good enough for you (at least as a temporary 
work-around). For Armv7-A or NCHW, nothing changes, we follow exactly the 
previous path.
   > 
   > For Armv8-A and NHWC we don't convert during the legalization step, but 
during the `_alter_conv2d_layout` pass. The only difference now is that the 
offset contribution will be added after the convolution instead than before.
   > 
   > I agree that a better solution, where the legalization changes depending 
on the strategy, would be better. However, I don't think the legalization step 
has got enough information to know the strategy (for now).
   > 
   > What do you think?
   
   I think it is ok.



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] giuseros commented on pull request #5754: [RFC] Improve quantized convolution performance for armv8 architectures

[GitBox]

giuseros commented on pull request #5754:
URL: https://github.com/apache/incubator-tvm/pull/5754#issuecomment-642682834


   Hi @FrozenGene , 
   I agree that different strategies should be available to the auto-tuner. See 
if the solution proposed is good enough for you (at least as a temporary 
work-around). For Armv7-A or NCHW, nothing changes, we follow exactly the 
previous path. 
   
   For Armv8-A and NHWC we don't convert during the legalization step, but 
during the `_alter_conv2d_layout` pass. The only difference now is that the 
offset contribution will be added after the convolution instead than before. 
   
   I agree that a better solution, where the legalization changes depending on 
the strategy, would be better. However, I don't think the legalization step has 
got enough information to know the strategy (for now). 
   
   What do you think?



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] FrozenGene commented on pull request #5754: [RFC] Improve quantized convolution performance for armv8 architectures

[GitBox]

FrozenGene commented on pull request #5754:
URL: https://github.com/apache/incubator-tvm/pull/5754#issuecomment-642671388


   @giuseros  I suddenly think of auto scheduler will have one environment 
value. So the change of legalization won't affect auto scheduler. We could 
check the value of this environment value for auto scheduler and use `smlal`. 
However, this problem I think we still should resolve that we should have the 
ability for allowing different strategies have different logic. 



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] FrozenGene commented on pull request #5754: [RFC] Improve quantized convolution performance for armv8 architectures

[GitBox]

FrozenGene commented on pull request #5754:
URL: https://github.com/apache/incubator-tvm/pull/5754#issuecomment-642651252


   > So I mean to add a `convert_data_type` pass that is similar to 
`alter_op_layout` but converts datatype (and we can do something like `if 
topi_impl == 'spatial_nhwc' converts to int16`.
   
   I think this is one interesting pass. Like we have `_alter_op_layout` and 
will have different logic for different strategy , then we have 
`_alter_op_dtype` pass and will have different logic for different strategy. 
   
   However, this pass seems do most of the same thing in legalize (change 
dtype).  So our legalization pass should complete this work according to 
different strategy. 



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] SJTU-yys commented on issue #5133: [Torch] A list of missing op conversion in need of help

[GitBox]

SJTU-yys commented on issue #5133:
URL: https://github.com/apache/incubator-tvm/issues/5133#issuecomment-642632617


   It seems that aten::norm is not  supported.
@siju-samuel  could you please add this layer? 



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] giuseros commented on pull request #5754: [RFC] Improve quantized convolution performance for armv8 architectures

[GitBox]

giuseros commented on pull request #5754:
URL: https://github.com/apache/incubator-tvm/pull/5754#issuecomment-642607696


   So I mean to add a `convert_data_type` pass that is similar to 
`alter_op_layout` but converts datatype (and we can do something like `if 
topi_impl == 'spatial_nhwc' converts to int16`. 
   
   This doesn't seem possible directly in the `alter_op_layout` because only 
the shapes are passed to that function, but I will play with it another bit
   
   To reply to your question, yes, the `alter_op_layout` pass is executed when 
the autotuner runs



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] masahi commented on pull request #5763: Fix gelu in PyTorch frontend, tighten numerical checks

[GitBox]

masahi commented on pull request #5763:
URL: https://github.com/apache/incubator-tvm/pull/5763#issuecomment-642603603


   Thanks @t-vi 



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

[incubator-tvm] branch master updated (e2fb503 -> 672e6cd)

[masahi]

This is an automated email from the ASF dual-hosted git repository.

masahi pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-tvm.git.


from e2fb503  [TOPI][RELAY][PYTORCH]Conv3d_transpose op support added 
(#5737)
 add 672e6cd  Fix gelu in PyTorch frontend, tighten numerical checks (#5763)

No new revisions were added by this update.

Summary of changes:
 python/tvm/relay/frontend/pytorch.py  | 20 
 tests/python/frontend/pytorch/test_forward.py | 27 ++-
 2 files changed, 26 insertions(+), 21 deletions(-)

[GitHub] [incubator-tvm] masahi merged pull request #5763: Fix gelu in PyTorch frontend, tighten numerical checks

[GitBox]

masahi merged pull request #5763:
URL: https://github.com/apache/incubator-tvm/pull/5763


   



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] FrozenGene commented on pull request #5754: [RFC] Improve quantized convolution performance for armv8 architectures

[GitBox]

FrozenGene commented on pull request #5754:
URL: https://github.com/apache/incubator-tvm/pull/5754#issuecomment-642601817


   > Hi @FrozenGene ,
   > 
   > The idea of adding the algorithm name to the attributes would work if the 
legalization step was run after we pick the strategy. It is instead run before, 
so it is unaware of the strategy picked. 
   > 
   > 
   > 
   > Maybe we could add a new pass that runs based on the strategy? Or we can 
hack in `_alter_conv2d_layout`? 
   > 
   > 
   
   @giuseros what you mean run based on the strategy?
   
   in alter_op_layout, we could extract workload[0] to get strategy, however 
could you help me to double check whether our autotvm tuning will use 
alter_op_layout pass?(i.e. O3), I have forgot a little bit. If so, maybe we 
could change the dtype here according to strategy. cc @anijain2305 any better 
idea too?
   



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] giuseros commented on pull request #5754: [RFC] Improve quantized convolution performance for armv8 architectures

[GitBox]

giuseros commented on pull request #5754:
URL: https://github.com/apache/incubator-tvm/pull/5754#issuecomment-642588670


   Hi @FrozenGene ,
   The idea of adding the algorithm name to the attributes would work if the 
legalization step was run after we pick the strategy. It is instead run before, 
so it is unaware of the strategy picked. 
   
   Maybe we could add a new pass that runs based on the strategy? Or we can 
hack in `_alter_conv2d_layout`? 
   



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] lixiaoquan commented on a change in pull request #5762: [TF] Support symbolic inputs of Fill

[GitBox]

lixiaoquan commented on a change in pull request #5762:
URL: https://github.com/apache/incubator-tvm/pull/5762#discussion_r438723482



##
File path: python/tvm/relay/frontend/tensorflow.py
##
@@ -1352,12 +1352,17 @@ def _impl(inputs, attr, params, mod):
 # Output shape must be defined to avoid errors. If any axis is not, we 
must
 # try to compute its shape.
 if output_shape is None or -1 in output_shape:
-output_shape = _infer_value(inputs[0], 
params).asnumpy().reshape([-1]).tolist()
+try:
+output_shape = _infer_value(inputs[0], 
params).asnumpy().reshape([-1]).tolist()

Review comment:
   It's 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:
us...@infra.apache.org

[GitHub] [incubator-tvm] FrozenGene commented on pull request #5754: [RFC] Improve quantized convolution performance for armv8 architectures

[GitBox]

FrozenGene commented on pull request #5754:
URL: https://github.com/apache/incubator-tvm/pull/5754#issuecomment-642577581


   > 1. It will be hard to do this. The point is that the legalization is done 
in Relay before picking the strategy (thus, it is unaware of the strategy 
picked). To keep both legalizations I need somehow to pass information from the 
strategy (e.g., the name of the algorithm, or something like that). Are you 
aware of any other ways I can do it?
   
   @giuseros I think add the algorithm name could be one way to handle it. For 
example, we could add it in the `attr` and query it in the legalization pass, 
then we could throw it safely.
   
   > Note that I am targeting NHWC layout. I wasn't able to even compile with 
conv2d_nhwc_spatial_pack for uint8 (it just hangs, at least when I tried it 
without auto-tuning on Armv8-A). I gathered from various discussions that NHWC 
support for arm targets is incomplete at the moment. So for now we might simply 
agree to leave this as default for NHWC and conv2d_nchw_spatial_pack as default 
for NCHW and mirror that in the legalization step which might look like:
   ```if is_aarch64_arm() and attrs.data_layout == "NHWC":
   return helper_change_dtypes_to_be_same(attrs, inputs, types, 
relay.qnn.op.conv2d)
   return helper_no_fast_int8_hw_legalization(attrs, inputs, types, 
relay.qnn.op.conv2d)```
   
   Yes, our NHWC schedule on arm cpu doesn't be complete. After our careful 
testing, NHWC is also perform better than NCHW on arm cpu using Ansor (aka auto 
scheduler) too. So this prompts us we could improve our AutoTVM NHWC schedule 
on arm cpu too.  As the result I show in the post,  we use auto schedule is to 
leverage NHWC layout and `smlal` instruction,  I prefer we could leverage 
`attr[algorithm_name]` mentioned previous to keep `smlal` instruction. After 
auto scheduler released (we are working hard to do it, we wish after 2 weeks we 
could bring it in), we could see how to improve it (like generating smlal and 
smlal2 or your tensorize instruction), they are orthogonal but they share the 
same legalize pass.
   
   One background of auto scheduler: In auto scheduler, we only need 
tvm.compute, then we could generate schedule automatically, so we could try 
NHWC / NCHW easily. So there is no spatial pack schedule template concept in 
the auto scheduler world in fact.
   
   > About the Raspberry+mobilenet v2, good to know you are working on Armv8-A 
(sorry to have assumed otherwise). However, there is still the point that 
mobilenet uses shallow convolutions, while I am addressing deeper and more 
generic convolutions.
   
   So we should keep both algorithm better, right?
   
   
   > Are you saying that, as things stand now in TVM, the 
conv2d_nhwc_spatial_pack schedule might be faster than the gemm approach on 
smaller CPUs? Unfortunately, for now I don't think they can be added together 
because of what I said above about the legalization step. Do you know any 
work-around to that? Maybe I can legalize only for specific devices (e.g., only 
for Cortex-A55)?
   
   I think add algorithm name mentioned before maybe could help to solve it.
   
   > Finally, as things stand now we might get this PR in, and later do a more 
detailed comparison across different networks + CPUs
   
   Ok. I buy it in. After legalization pass we discussed is solved, I am glad 
to do code review carefully and handle this 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:
us...@infra.apache.org

[GitHub] [incubator-tvm] giuseros commented on pull request #5754: [RFC] Improve quantized convolution performance for armv8 architectures

[GitBox]

giuseros commented on pull request #5754:
URL: https://github.com/apache/incubator-tvm/pull/5754#issuecomment-642564985


   Hi @FrozenGene 
   Just to clarify: I am enjoying the discussion, and since the optimization 
space is wild, I agree that is worth valuating different approaches. 
   * About the Raspberry+mobilenet v2, good to know you are working on Armv8-A 
(sorry to have assumed otherwise). However, there is still the point that 
mobilenet uses shallow convolutions, while I am addressing deeper and more 
generic convolutions. 
   * Are you saying that, as things stand now in TVM, the 
`conv2d_nhwc_spatial_pack` schedule might be faster than the gemm approach on 
smaller CPUs? Unfortunately, for now I don't think they can be added together 
because of what I said above about the legalization step. Do you know any 
work-around to that? Maybe I can legalize only for specific devices (e.g., only 
for Cortex-A55)? 
   * Finally, as things stand now we might get this PR in, and later do a more 
detailed comparison across different networks + CPUs



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] giuseros edited a comment on pull request #5754: [RFC] Improve quantized convolution performance for armv8 architectures

[GitBox]

giuseros edited a comment on pull request #5754:
URL: https://github.com/apache/incubator-tvm/pull/5754#issuecomment-642520722


   Hi @FrozenGene ,
   Thanks a lot for your comments.  I will address general replies here, and 
code comments in a separate reply.
   
   * I indeed read your discuss 
[post](https://discuss.tvm.ai/t/tflite-and-tvm-comparison-for-quantized-models/6577/4),
 but I thought the work was orthogonal to this one. My main goal here is to 
have a fast general convolution algorithm for Armv8-A. Your post talks about 
mobilenet v2, and raspi 3. 
   * In mobilenet v2 there are no deep convolutional layers, mostly depthwise 
convolutions and 1x1 convolutions. With shallow convolutions the problem 
becomes memory bound, and the differences among the algorithms  become less 
evident. That is also why I picked inception_v3, where there are 1x1, 3x3, 5x5, 
1x7, 7x1 convolutions. 
   * Raspi 3 comes with a 32bit operative system, which means using Armv7-A. 
The problem with Armv7-A is that instead of having 32 registers (as in Armv8-A) 
you have only 16, so the optimization space is reduced. Also, I think (but I am 
not 100% sure) that the guys in TFlite do not extremely optimize for Armv7-A. 
Indeed, on Armv7-A @anijain2305 shows (in the same post you mention) a 0.80 
ratio for tflite/tvm (while I see a 0.60/0.30 ratio for multi/single thread 
scenarios, respectively ). 
   * The Qnnpack post you mention explicitly says that: "the microkernel that 
leverages the dual issue capability proves to be 15 percent to 20 percent 
faster for a sufficiently large channel count (K > 64)"
   * The way they do convolution (and gemm) in Qnnpack for Armv8-A is by using 
a combination of `smlal` and `smlal2` (plus a combination of `usubl` and 
`usubl2`) while `conv2d_nhwc_spatial_pack` only uses `smal`. It is true that in 
Armv7-A they only use `vsmal` (and `vusubl`). So, I wonder if the autoscheduler 
(which I am not familiar with) is able to generate such combinations for armv8. 
   * I did not try other CPUs other than the Cortex-A76. The point is that I am 
not using anything specific for that CPU, but only specific to the Armv8-A ISA. 
 
   * I agree that in case of smaller convolutions (or depthwise convolutions) 
there are simpler algorithms that work as well (or even faster). I also agree 
in stacking multiple strategies and let TVM select the best. 
   
   I will reply on the code in the following comment. 



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] giuseros commented on pull request #5754: [RFC] Improve quantized convolution performance for armv8 architectures

[GitBox]

giuseros commented on pull request #5754:
URL: https://github.com/apache/incubator-tvm/pull/5754#issuecomment-642543391


   Hi @FrozenGene ,
   About the code changes. 
   1) It will be hard to do this. The point is that the legalization is done in 
Relay before picking the strategy (thus, it is unaware of the strategy picked). 
To keep both legalizations I need somehow to pass information from the strategy 
(e.g., the name of the algorithm, or something like that). Are you aware of any 
other ways I can do it?

   2) Note that I am targeting NHWC layout. I wasn't able to even compile with 
`conv2d_nhwc_spatial_pack`  for uint8 (it just hangs, at least when I tried it 
without auto-tuning on Armv8-A). I gathered from various discussions that NHWC 
support for arm targets is incomplete at the moment. So for now we might simply 
agree to leave this as default for NHWC and `conv2d_nchw_spatial_pack` as 
default for NCHW and mirror that in the legalization step which might look like:
   
   ```def _qnn_conv2d_legalize_arm_cpu(attrs, inputs, types):
   if is_aarch64_arm() and attrs.data_layout == "NHWC":
   return helper_change_dtypes_to_be_same(attrs, inputs, types, 
relay.qnn.op.conv2d)
   return helper_no_fast_int8_hw_legalization(attrs, inputs, types, 
relay.qnn.op.conv2d)```
   
   In a subsequent work, we can find a way to pick the correct legalization 
after we picked the strategy.



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] FrozenGene edited a comment on pull request #5754: [RFC] Improve quantized convolution performance for armv8 architectures

[GitBox]

FrozenGene edited a comment on pull request #5754:
URL: https://github.com/apache/incubator-tvm/pull/5754#issuecomment-642541198


   @giuseros  Glad to see we have the same thought we should let autotvm select 
the best.
   
   Autoscheduler reley on the legalization pass to generate smlal inst(After 
auto scheduler is released, let us make it better together.) One information I 
missed before, my testing rasp 3b+ os is Ubuntu 64 bits, not 32 bits, so the 
target is aarch64 too. 
   
   I mention auto scheduler is not to question your work (your work is very 
great!) and is orthogonal as you said. I just mention that we use smlal inst on 
A53 (aarch64 os mentioned before) we could get nice performance too. So I want 
to know on low-end arm cpu, whether smlal is better than this (as fb qnnpack 
blog said: The default microkernel uses the fewest possible instructions and 
thus delivers the best performance on low-end cores, which can execute only one 
NEON instruction per cycle.).
   
   So I wish we could test several arm cpus to proove our this work work well 
all aarch64 cores (low-end core, high-end core).
   
   Secondly, I suggest let us test mobilenet v2 too. To see that whether our pr 
could work well across various models. 
   
   Your work is very great but I wish let us use more data and result to make 
it more convincing. 
   



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] FrozenGene commented on pull request #5754: [RFC] Improve quantized convolution performance for armv8 architectures

[GitBox]

FrozenGene commented on pull request #5754:
URL: https://github.com/apache/incubator-tvm/pull/5754#issuecomment-642541198


   Glad to see we have the same thought we should let autotvm select the best.
   
   Autoscheduler reley on the legalization pass to generate smlal inst(After 
auto scheduler is released, let us make it better together.) One information I 
missed before, my testing rasp 3b+ os is Ubuntu 64 bits, not 32 bits, so the 
target is aarch64 too. 
   
   I mention auto scheduler is not to question your work (your work is very 
great!) and is orthogonal as you said. I just mention that we use smlal inst on 
A53 (aarch64 os mentioned before) we could get nice performance too. So I want 
to know on low-end arm cpu, whether smlal is better than this (as fb qnnpack 
blog said: The default microkernel uses the fewest possible instructions and 
thus delivers the best performance on low-end cores, which can execute only one 
NEON instruction per cycle.).
   
   So I wish we could test several arm cpus to proove our this work work well 
all aarch64 cores (low-end core, high-end core).
   
   Secondly, I suggest let us test mobilenet v2 too. To see that whether our pr 
could work well across various models. 
   
   Your work is very great but I wish let us use more data and result to make 
it more convincing. 
   



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] giuseros edited a comment on pull request #5754: [RFC] Improve quantized convolution performance for armv8 architectures

[GitBox]

giuseros edited a comment on pull request #5754:
URL: https://github.com/apache/incubator-tvm/pull/5754#issuecomment-642520722


   Hi @FrozenGene ,
   Thanks a lot for your comments.  I will address general replies here, and 
code comments in a separate reply.
   
   * I indeed read your discuss 
[post](https://discuss.tvm.ai/t/tflite-and-tvm-comparison-for-quantized-models/6577/4),
 but I thought the work was orthogonal to this one. My main goal here is to 
have a fast general convolution algorithm for Armv8-A. Your post talks about 
mobilenet v2, and raspi 3. 
   * In mobilenet v2 there are no deep convolutional layers, mostly depthwise 
convolutions and 1x1 convolutions. With shallow convolutions the problem 
becomes memory bound, and the differences among the algorithms  become less 
evident. That is also why I picked inception_v3, where there are 1x1, 3x3, 5x5, 
1x7, 7x1 convolutions. 
   * Raspi 3 comes with a 32bit operative system, which means using Armv7-A. 
The problem with Armv7-A is that instead of having 32 registers (as in Armv8-A) 
you have only 16, so the optimization space is reduced. Also, I think (but I am 
not 100% sure) that the guys in TFlite do not extremely optimize for Armv7-A. 
Indeed, on Armv7-A @anijain2305 shows (in the same post you mention) a 0.80 
ratio for tflite/tvm (while I see a 0.60/0.30 ratio for multi/single thread 
scenarios, respectively ). 
   * The Qnnpack post you mention explicitly says that: "the microkernel that 
leverages the dual issue capability proves to be 15 percent to 20 percent 
faster for a sufficiently large channel count (K > 64)"
   * The way they do convolution (and gemm) in Qnnpack for Armv8-A is by using 
a combination of `smlal` and `smlal2` (plus a combination of `usubl` and 
`usubl2`) while `conv2d_nhwc_spatial_pack` only uses `smal`. It is true that in 
armv7 they only use `vsmal` (and `vusubl`). So, I wonder if the autoscheduler 
(which I am not familiar with) is able to generate such combinations for armv8. 
   * I did not try other CPUs other than the Cortex-A76. The point is that I am 
not using anything specific for that CPU, but only specific to the Armv8-A ISA. 
 
   * I agree that in case of smaller convolutions (or depthwise convolutions) 
there are simpler algorithms that work as well (or even faster). I also agree 
in stacking multiple strategies and let TVM select the best. 
   
   I will reply on the code in the following comment. 



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] giuseros edited a comment on pull request #5754: [RFC] Improve quantized convolution performance for armv8 architectures

[GitBox]

giuseros edited a comment on pull request #5754:
URL: https://github.com/apache/incubator-tvm/pull/5754#issuecomment-642520722


   Hi @FrozenGene ,
   Thanks a lot for your comments.  I will address general replies here, and 
code comments in a separate reply.
   
   * I indeed read your discuss 
[post](https://discuss.tvm.ai/t/tflite-and-tvm-comparison-for-quantized-models/6577/4),
 but I thought the work was orthogonal to this one. My main goal here is to 
have a fast general convolution algorithm for armv8. Your post talks about 
mobilenet v2, and raspi 3. 
   * In mobilenet v2 there are no deep convolutional layers, mostly depthwise 
convolutions and 1x1 convolutions. With shallow convolutions the problem 
becomes memory bound, and the differences among the algorithms  become less 
evident. That is also why I picked inception_v3, where there are 1x1, 3x3, 5x5, 
1x7, 7x1 convolutions. 
   * Raspi 3 comes with a 32bit operative system, which means using armv7. The 
problem with armv7 is that instead of having 32 registers (as in armv8) you 
have only 16, so the optimization space is reduced. Also, I think (but I am not 
100% sure) that the guys in TFlite do not extremely optimize for armv7. Indeed, 
on armv7 @anijain2305 shows (in the same post you mention) a 0.80 ratio for 
tflite/tvm (while I see a 0.60/0.30 ratio for multi/single thread scenarios, 
respectively ). 
   * The Qnnpack post you mention explicitly says that: "the microkernel that 
leverages the dual issue capability proves to be 15 percent to 20 percent 
faster for a sufficiently large channel count (K > 64)"
   * The way they do convolution (and gemm) in Qnnpack for armv8 is by using a 
combination of `smlal` and `smlal2` (plus a combination of `usubl` and 
`usubl2`) while `conv2d_nhwc_spatial_pack` only uses `smal`. It is true that in 
armv7 they only use `vsmal` (and `vusubl`). So, I wonder if the autoscheduler 
(which I am not familiar with) is able to generate such combinations for armv8. 
   * I did not try other CPUs other than the Cortex-A76. The point is that I am 
not using anything specific for that CPU, but only specific to the armv8 ISA.  
   * I agree that in case of smaller convolutions (or depthwise convolutions) 
there are simpler algorithms that work as well (or even faster). I also agree 
in stacking multiple strategies and let TVM select the best. 
   
   I will reply on the code in the following comment. 



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] giuseros commented on pull request #5754: [RFC] Improve quantized convolution performance for armv8 architectures

[GitBox]

giuseros commented on pull request #5754:
URL: https://github.com/apache/incubator-tvm/pull/5754#issuecomment-642520722


   Hi @FrozenGene ,
   Thanks a lot for your comments.  I will address general replies here, and 
code comments in a separate reply.
   
   * I indeed read your discuss 
[post](https://discuss.tvm.ai/t/tflite-and-tvm-comparison-for-quantized-models/6577/4),
 but I thought the work was orthogonal to this one. My main goal here is to 
have a fast general convolution algorithm for armv8. Your post talks about 
mobilenet v2, and raspi 3. 
   * In mobilenet v2 there are no deep convolutional layers, mostly depthwise 
convolutions and 1x1 convolutions. With shallow convolutions the problem 
becomes memory bound, and the differences among the algorithms  become less 
evident. That is also why I picked inception_v3, where there are 1x1, 3x3, 5x5, 
1x7, 7x1 convolutions. 
   * Raspi 3 comes with a 32bit operative system, which means using armv7. The 
problem with armv7 is that instead of having 32 registers (as in armv8) you 
have only 16, so the optimization space is reduced. Also, I think (but I am not 
100% sure) that the guys in TFlite do not extremely optimize for armv7. Indeed, 
on armv7 @anijain2305 shows (in the same post you mention) a 0.80 ratio for 
tflite/tvm (while I see a 0.60/0.30 ratio for multi/single thread scenarios, 
respectively ). 
   * The Qnnpack post you mention explicitly says that: "the microkernel that 
leverages the dual issue capability proves to be 15 percent to 20 percent 
faster for a sufficiently large channel count (K > 64)"
   * The way they do convolution (and gemm) in Qnnpack for armv8 is by using a 
combination of `smlal` and `smlal2` (plus a combination of `usubl` and 
`usubl2`) while `conv2d_nhwc_spatial_pack` only uses `smal`. It is true that in 
armv7 they only use `vsmal` (and `vusubl`). So, I wonder if the autoscheduler 
(which I am not familiar with) is able to generate such combinations for armv8. 
   * I did not try other CPUs other than the a76. The point is that I am not 
using anything specific for that CPU, but only specific to the armv8 ISA.  
   * I agree that in case of smaller convolutions (or depthwise convolutions) 
there are simpler algorithms that work as well (or even faster). I also agree 
in stacking multiple strategies and let TVM select the best. 
   
   I will reply on the code in the following comment. 



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] masahi commented on pull request #5737: [TOPI][RELAY][PYTORCH]Conv3d_transpose op support added

[GitBox]

masahi commented on pull request #5737:
URL: https://github.com/apache/incubator-tvm/pull/5737#issuecomment-642520252


   @siju-samuel 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:
us...@infra.apache.org

[incubator-tvm] branch master updated (c7274fd -> e2fb503)

[masahi]

This is an automated email from the ASF dual-hosted git repository.

masahi pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-tvm.git.


from c7274fd  [Relay] Fix for recursive let (#5757)
 add e2fb503  [TOPI][RELAY][PYTORCH]Conv3d_transpose op support added 
(#5737)

No new revisions were added by this update.

Summary of changes:
 docs/api/python/topi.rst   |   4 +
 docs/langref/relay_op.rst  |   4 +-
 include/tvm/relay/attrs/nn.h   |  76 +
 python/tvm/relay/frontend/pytorch.py   |  21 ++-
 python/tvm/relay/op/nn/_nn.py  |  25 +++
 python/tvm/relay/op/nn/nn.py   |  70 +
 python/tvm/relay/op/op_attrs.py|   3 +
 python/tvm/relay/op/strategy/cuda.py   |  18 +++
 python/tvm/relay/op/strategy/generic.py|  38 +
 python/tvm/relay/op/strategy/x86.py|  18 +++
 src/relay/op/nn/convolution.cc |  45 ++
 src/relay/op/nn/convolution.h  |  97 
 tests/python/frontend/pytorch/test_forward.py  |  28 
 tests/python/relay/test_op_level2.py   |  62 
 topi/python/topi/cuda/__init__.py  |   1 +
 topi/python/topi/cuda/conv3d.py|   2 +-
 topi/python/topi/cuda/conv3d_transpose_ncdhw.py| 145 ++
 topi/python/topi/generic/nn.py |  18 +++
 topi/python/topi/nn/__init__.py|   1 +
 topi/python/topi/nn/conv3d_transpose.py| 169 +
 topi/python/topi/testing/__init__.py   |   1 +
 .../topi/testing/conv3d_transpose_ncdhw_python.py  |  86 +++
 topi/python/topi/x86/__init__.py   |   1 +
 .../{conv2d_transpose.py => conv3d_transpose.py}   |  23 +--
 ...nchw.py => test_topi_conv3d_transpose_ncdhw.py} |  62 
 25 files changed, 967 insertions(+), 51 deletions(-)
 create mode 100644 topi/python/topi/cuda/conv3d_transpose_ncdhw.py
 create mode 100644 topi/python/topi/nn/conv3d_transpose.py
 create mode 100644 topi/python/topi/testing/conv3d_transpose_ncdhw_python.py
 copy topi/python/topi/x86/{conv2d_transpose.py => conv3d_transpose.py} (74%)
 copy topi/tests/python/{test_topi_conv2d_transpose_nchw.py => 
test_topi_conv3d_transpose_ncdhw.py} (50%)

[GitHub] [incubator-tvm] masahi merged pull request #5737: [TOPI][RELAY][PYTORCH]Conv3d_transpose op support added

[GitBox]

masahi merged pull request #5737:
URL: https://github.com/apache/incubator-tvm/pull/5737


   



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] leandron opened a new issue #5774: [TFLite] KeyError: 'conv2d/Kernel' on test_forward_mediapipe_hand_landmark()

[GitBox]

leandron opened a new issue #5774:
URL: https://github.com/apache/incubator-tvm/issues/5774


   After #5682, we're now seeing the error below on 
`test_forward_mediapipe_hand_landmark`. I think that it is not seen on upstream 
CI because of file caching. As I ran this test, master points at 
`c7274fd3b0f693fc6214a450e13d5e99026337ae`.
   
   This is the error:
   
   ```
   _ test_forward_mediapipe_hand_landmark 
_
   
   def test_forward_mediapipe_hand_landmark():
   """Test MediaPipe 2D hand landmark TF Lite model."""
   # MediaPipe 2D hand landmark TF
   tflite_model_file = download_testdata(
   
"https://github.com/google/mediapipe/raw/master/mediapipe/models/hand_landmark.tflite;,
   "hand_landmark.tflite")
   with open(tflite_model_file, "rb") as f:
   tflite_model_buf = f.read()
   data = np.random.uniform(size=(1, 256, 256, 3)).astype('float32')
   tflite_output = run_tflite_graph(tflite_model_buf, data)
   >   tvm_output = run_tvm_graph(tflite_model_buf, data, 'input_1', 
num_output=2)
   
   tests/python/frontend/tflite/test_forward.py:2451: 
   _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
_ _ 
   tests/python/frontend/tflite/test_forward.py:136: in run_tvm_graph
   dtype_dict=dtype_dict)
   python/tvm/relay/frontend/tflite.py:2712: in from_tflite
   op_converter.convert_op_to_relay()
   python/tvm/relay/frontend/tflite.py:185: in convert_op_to_relay
   ret = self.convert_map[op_code_str](op)
   python/tvm/relay/frontend/tflite.py:2454: in convert_dequantize
   in_expr = self.get_expr(input_tensor.tensor_idx)
   python/tvm/relay/frontend/tflite.py:2563: in get_expr
   return self.exp_tab.get_expr(get_tensor_name(self.subgraph, 
input_tensor_idx))
   _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
_ _ 
   
   self = 
   name = 'conv2d/Kernel'
   
   def get_expr(self, name):
   >   return self.exprs[name]
   E   KeyError: 'conv2d/Kernel'
   
   python/tvm/relay/frontend/common.py:294: KeyError
   - Captured stdout call 
-
   ```



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] maheshambule commented on pull request #5495: [Relay, Topi] [Frontend][TFLite, MXNet] ReverseSequence operator

[GitBox]

maheshambule commented on pull request #5495:
URL: https://github.com/apache/incubator-tvm/pull/5495#issuecomment-642517734


   @siju-samuel, rebased to master. PTAL.



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] maheshambule commented on a change in pull request #5052: [TARGET] ONNX codegen

[GitBox]

maheshambule commented on a change in pull request #5052:
URL: https://github.com/apache/incubator-tvm/pull/5052#discussion_r438627390



##
File path: tests/python/contrib/test_onnx.py
##
@@ -0,0 +1,467 @@
+# 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.
+
+"""Relay to ONNX serialization test cases"""
+import pytest
+pytest.importorskip('onnx')
+pytest.importorskip('onnxruntime')
+
+import numpy as np
+import onnxruntime as rt
+
+import tvm
+from tvm import relay
+from tvm.contrib.target.onnx import to_onnx
+
+
+
+def func_to_onnx(func, name):
+mod = tvm.IRModule()
+mod['main'] = func
+onnx_model = to_onnx(mod, {}, name, path=None)
+return onnx_model.SerializeToString()
+
+
+def run_onnx(onnx_model, input_data):
+sess = rt.InferenceSession(onnx_model)
+input_names = {}
+for input, data in zip(sess.get_inputs(), input_data):
+input_names[input.name] = data
+output_names = [out.name for out in sess.get_outputs()]
+res = sess.run(output_names, input_names)
+return res
+
+
+def run_relay(func, data_tuple):
+target = 'llvm'
+ctx = tvm.context('llvm', 0)
+intrp = relay.create_executor("graph", ctx=ctx, target=target)
+relay_res = intrp.evaluate(func)(*data_tuple)
+
+result = []
+relay_res = relay_res if isinstance(relay_res, list) else [relay_res]
+for res in relay_res:
+result.append(res.asnumpy())
+
+return result
+
+
+def verify_results(relay_func, indata, test_name, rtol=1e-7, atol=0):
+relay_results = run_relay(relay_func, indata)
+onnx_results = run_onnx(func_to_onnx(relay_func, test_name), indata)
+
+for relay_res, onnx_res in zip(relay_results, onnx_results):
+np.testing.assert_allclose(relay_res, onnx_res, rtol=rtol, atol=atol)
+
+
+def test_add():
+dtype = 'float32'
+t1 = relay.TensorType((5, 10, 5))
+t2 = relay.TensorType((5, 10, 5))
+x = relay.var("x", t1, dtype=dtype)
+y = relay.var("y", t2, dtype=dtype)
+z = relay.add(x, y)
+func = relay.Function([x, y], z)
+
+x_data = np.random.rand(5, 10, 5).astype(dtype)
+y_data = np.random.rand(5, 10, 5).astype(dtype)
+
+verify_results(func, [x_data, y_data], 'test_add')
+
+
+def test_bias_add():
+for dtype in ['float16', 'float32']:
+xshape = (10, 2, 3, 4)
+bshape = (2,)
+rtol = 1e-2 if dtype == 'float16' else 1e-5
+x = relay.var("x", shape=xshape, dtype=dtype)
+bias = relay.var("bias", dtype=dtype)
+z = relay.nn.bias_add(x, bias)
+func = relay.Function([x, bias], z)
+
+x_data = np.random.uniform(size=xshape).astype(dtype)
+y_data = np.random.uniform(size=bshape).astype(dtype)
+
+verify_results(func, [x_data, y_data], 'test_bias_add', rtol=rtol)
+
+
+def test_conv2d():
+def verify_conv2d(dtype, scale, dshape, kshape,
+  padding=(1, 1),
+  groups=1,
+  dilation=(1, 1),
+  **attrs):
+x = relay.var("x", shape=dshape, dtype=dtype)
+w = relay.var("w", shape=kshape, dtype=dtype)
+y = relay.nn.conv2d(x, w,
+padding=padding,
+dilation=dilation,
+groups=groups,
+**attrs)
+func = relay.Function([x, w], y)
+data = np.random.uniform(-scale, scale, size=dshape).astype(dtype)
+kernel = np.random.uniform(-scale, scale, size=kshape).astype(dtype)
+verify_results(func, [data, kernel], 'test_conv2d', rtol=1e-5, 
atol=1e-5)
+
+dshape = (1, 32, 18, 18)
+kshape = (32, 1, 3, 3)
+verify_conv2d("float32", 1, dshape, kshape,
+  padding=(1, 1), channels=32, groups=32, kernel_size=(3, 3))
+
+dshape = (1, 32, 18, 18)
+kshape = (32, 4, 3, 3)
+verify_conv2d("float32", 1, dshape, kshape,
+  padding=(1, 1), channels=32, groups=8, kernel_size=(3, 3))
+
+# also group conv2d
+dshape = (1, 32, 18, 18)
+kshape = (64, 1, 3, 3)
+verify_conv2d("float32", 1, dshape, kshape,
+  padding=(1, 1), channels=64, groups=32, kernel_size=(3, 3))
+
+# normal conv2d
+dshape = (1, 3, 

[GitHub] [incubator-tvm] maheshambule commented on a change in pull request #5052: [TARGET] ONNX codegen

[GitBox]

maheshambule commented on a change in pull request #5052:
URL: https://github.com/apache/incubator-tvm/pull/5052#discussion_r438627012



##
File path: python/tvm/contrib/target/onnx.py
##
@@ -0,0 +1,905 @@
+# 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, import-self, len-as-condition, 
unused-argument, too-many-lines, redefined-builtin
+"""Relay to ONNX codegen """
+
+import os
+import struct
+import copy
+import numpy
+import onnx
+import onnx.utils
+from onnx import numpy_helper, OperatorSetIdProto, defs
+import tvm
+from tvm import relay
+import tvm._ffi
+from tvm.relay.expr_functor import ExprVisitor
+from tvm.relay.ty import TupleType, TensorType
+
+ONNX_OPSET_VERSONS_SUPPORTED = [11]
+
+
+def tvm_array_to_list(arr):
+return tuple(x.value for x in arr)
+
+
+def get_onnx_version():
+return onnx.__version__
+
+
+def infer_type(node):
+"""A method to infer the type of a relay expression."""
+mod = tvm.IRModule.from_expr(node)
+mod = relay.transform.InferType()(mod)
+entry = mod["main"]
+return entry if isinstance(node, relay.Function) else entry.body
+
+
+def call_node_infer_type(node):
+"""infer the output types of call node"""
+infer_out = infer_type(node)
+out_type = infer_out._checked_type_
+types = []
+if isinstance(out_type, TensorType):
+types.append(out_type)
+elif isinstance(out_type, TupleType):
+for tupe_type in out_type.fields:
+types.append(tupe_type)

Review comment:
   changed.





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