[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5030: [RELAY] Added a AnnotatedRegion utility class

2020-03-24 Thread GitBox
zhiics commented on a change in pull request #5030: [RELAY] Added a 
AnnotatedRegion utility class
URL: https://github.com/apache/incubator-tvm/pull/5030#discussion_r397261539
 
 

 ##
 File path: python/tvm/relay/analysis/annotated_regions.py
 ##
 @@ -0,0 +1,62 @@
+# 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=no-else-return, unidiomatic-typecheck, invalid-name, 
unused-import
+"""Regions used in Relay."""
+
+from tvm.runtime import Object
+from . import _ffi_api
+
+
+class AnnotatedRegionSet(Object):
+"""Class to represent a relay expression split into regions."""
+
+def __init__(self, expr, region_begin_op, region_end_op):
+"""Construct regions from an expression.
+
+Parameters
+--
+expr : tvm.relay.Expr
+The expression from which to construct the regions.
+region_begin_op : tvm.relay.Op
+The region begin annotation.
+region_end_op : tvm.relay.Op
+The region end annotation.
+
+"""
+self.__init_handle_by_constructor__(_ffi_api.AnnotatedRegionSet,
+expr,
+region_begin_op,
+region_end_op)
+
+def __len__(self):
+return len(self.regions)
+
+def get_region(self, expr):
+"""Get the region an expression belongs to.
+
+Parameters
+--
+expr : tvm.relay.Expr
+The expression.
+
+Returns
+---
+region : Region
 
 Review comment:
   Yes, but without defining the class. Sphinx is not able to generate the 
correct documentation for it. It is basically an undefined symbol.


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


With regards,
Apache Git Services


[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5030: [RELAY] Added a AnnotatedRegion utility class

2020-03-23 Thread GitBox
zhiics commented on a change in pull request #5030: [RELAY] Added a 
AnnotatedRegion utility class
URL: https://github.com/apache/incubator-tvm/pull/5030#discussion_r396765779
 
 

 ##
 File path: python/tvm/relay/analysis/annotated_regions.py
 ##
 @@ -0,0 +1,62 @@
+# 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=no-else-return, unidiomatic-typecheck, invalid-name, 
unused-import
+"""Regions used in Relay."""
+
+from tvm.runtime import Object
+from . import _ffi_api
+
+
+class AnnotatedRegionSet(Object):
+"""Class to represent a relay expression split into regions."""
+
+def __init__(self, expr, region_begin_op, region_end_op):
+"""Construct regions from an expression.
+
+Parameters
+--
+expr : tvm.relay.Expr
+The expression from which to construct the regions.
+region_begin_op : tvm.relay.Op
+The region begin annotation.
+region_end_op : tvm.relay.Op
+The region end annotation.
+
+"""
+self.__init_handle_by_constructor__(_ffi_api.AnnotatedRegionSet,
+expr,
+region_begin_op,
+region_end_op)
+
+def __len__(self):
+return len(self.regions)
+
+def get_region(self, expr):
+"""Get the region an expression belongs to.
+
+Parameters
+--
+expr : tvm.relay.Expr
+The expression.
+
+Returns
+---
+region : Region
 
 Review comment:
   Yeah, I didn't see it is exposed or defined in Python. Therefore, I 
mentioned if we need to have `AnnotatedRegion/Node` as object nodes in C++ as 
well.


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


With regards,
Apache Git Services


[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5030: [RELAY] Added a AnnotatedRegion utility class

2020-03-23 Thread GitBox
zhiics commented on a change in pull request #5030: [RELAY] Added a 
AnnotatedRegion utility class
URL: https://github.com/apache/incubator-tvm/pull/5030#discussion_r396765779
 
 

 ##
 File path: python/tvm/relay/analysis/annotated_regions.py
 ##
 @@ -0,0 +1,62 @@
+# 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=no-else-return, unidiomatic-typecheck, invalid-name, 
unused-import
+"""Regions used in Relay."""
+
+from tvm.runtime import Object
+from . import _ffi_api
+
+
+class AnnotatedRegionSet(Object):
+"""Class to represent a relay expression split into regions."""
+
+def __init__(self, expr, region_begin_op, region_end_op):
+"""Construct regions from an expression.
+
+Parameters
+--
+expr : tvm.relay.Expr
+The expression from which to construct the regions.
+region_begin_op : tvm.relay.Op
+The region begin annotation.
+region_end_op : tvm.relay.Op
+The region end annotation.
+
+"""
+self.__init_handle_by_constructor__(_ffi_api.AnnotatedRegionSet,
+expr,
+region_begin_op,
+region_end_op)
+
+def __len__(self):
+return len(self.regions)
+
+def get_region(self, expr):
+"""Get the region an expression belongs to.
+
+Parameters
+--
+expr : tvm.relay.Expr
+The expression.
+
+Returns
+---
+region : Region
 
 Review comment:
   Yeah, I didn't see it is exposed or defined in Python. Therefore, I 
mentioned if we need to have `AnnotatedRegion/Node` as C++ as well.


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


With regards,
Apache Git Services


[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5030: [RELAY] Added a AnnotatedRegion utility class

2020-03-23 Thread GitBox
zhiics commented on a change in pull request #5030: [RELAY] Added a 
AnnotatedRegion utility class
URL: https://github.com/apache/incubator-tvm/pull/5030#discussion_r396652760
 
 

 ##
 File path: src/relay/analysis/annotated_region_set.cc
 ##
 @@ -0,0 +1,239 @@
+/*
+ * 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.
+ */
+
+#include "annotated_region_set.h"
+
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+
+namespace tvm {
+namespace relay {
+
+AnnotatedRegion AnnotatedRegionSetNode::GetRegion(const Expr& expr) const {
+  for (auto candidate : regions_) {
+if (candidate->nodes.find(expr) != candidate->nodes.end()) {
+  return candidate;
+}
+  }
+  return AnnotatedRegion(nullptr);
+}
+
+void AnnotatedRegionSetNode::MergeRegions(AnnotatedRegion region1,
+  AnnotatedRegion region2) {
+  if (region1 == region2) {
+return;
+  }
+
+  // Merge region 2 to region 1 and erase region 2.
+  region1->nodes.insert(region2->nodes.begin(), region2->nodes.end());
+  for (auto arg : region2->ins) {
+region1->ins.push_back(arg);
+  }
+  for (auto out : region2->outs) {
+region1->outs.push_back(out);
+  }
+  // if any of the outputs of 2 are inputs of 1, they become internal nodes
+  // so remove them from outs/args
+  std::vector args_to_remove;
+  for (const auto& arg : region1->ins) {
+auto call = Downcast(arg);
+auto it = std::find(region2->outs.begin(), region2->outs.end(), 
call->args[0]);
+if (it != region2->outs.end()) {
+  args_to_remove.push_back(arg);
+  region1->outs.remove(*it);
+}
+  }
+  for (const auto& arg : args_to_remove) {
+region1->ins.remove(arg);
+  }
+  regions_.erase(region2);
+}
+
+void AnnotatedRegionSetNode::AddToRegion(AnnotatedRegion region, const Expr& 
expr) {
+  auto region2 = GetRegion(expr);
+  if (region2.defined()) {
+MergeRegions(region, region2);
+  } else {
+region->nodes.insert(expr);
+  }
+}
+
+AnnotatedRegion AnnotatedRegionSetNode::MakeRegion() {
+  auto ret = regions_.emplace(AnnotatedRegion());
+  return *ret.first;
+}
+
+class AnnotatedRegionSet::Creator : public ExprVisitor {
+ public:
+  Creator(const Op _begin_op, const Op _end_op) :
 
 Review comment:
   code style: Op& region_begin_op
   
   Many other places in this file as well, please put `*` together with the 
type instead of the variables/parameters to keep consistent to the Google C++ 
code style.


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


With regards,
Apache Git Services


[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5030: [RELAY] Added a AnnotatedRegion utility class

2020-03-23 Thread GitBox
zhiics commented on a change in pull request #5030: [RELAY] Added a 
AnnotatedRegion utility class
URL: https://github.com/apache/incubator-tvm/pull/5030#discussion_r396646449
 
 

 ##
 File path: src/relay/analysis/annotated_region_set.h
 ##
 @@ -0,0 +1,277 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/relay/pass/annotated_region_set.h
+ * \brief Define data structures to extract and manipulate regions from
+ * a relay function. Regions are denoted by region_begin and region_end
+ * annotations that exist on all the input and output edges of the region.
+ */
+
+#ifndef TVM_RELAY_ANALYSIS_ANNOTATED_REGION_SET_H_
+#define TVM_RELAY_ANALYSIS_ANNOTATED_REGION_SET_H_
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace tvm {
+namespace relay {
+
+class AnnotatedRegion;
+class AnnotatedRegionSet;
+
+class AnnotatedRegionNode : public Object {
+ public:
+  void VisitAttrs(AttrVisitor* v) {
+v->Visit("id", );
+Array nodes_array(nodes.begin(), nodes.end());
+v->Visit("nodes", _array);
+Array args_array(ins.begin(), ins.end());
+v->Visit("args", _array);
+Array rets_array(outs.begin(), outs.end());
+v->Visit("rets", _array);
+  }
+
+  /*! \brief Get the region ID. */
+  int GetID() const {
+return id;
+  }
+
+  /*! \brief Get the region's inputs. */
+  std::list GetInputs() const {
+return ins;
+  }
+
+  /*! \brief Get the region's outputs. */
+  std::list GetOutputs() const {
+return outs;
+  }
+
+  /*! \brief Get the region's nodes. */
+  std::unordered_set GetNodes() const {
+return nodes;
+  }
+
+  static constexpr const char* _type_key = "relay.AnnotatedRegion";
+  TVM_DECLARE_FINAL_OBJECT_INFO(AnnotatedRegionNode, Object);
+
+ protected:
+  /*! \brief The region ID. */
+  int id{-1};
+  /*! \brief The inputs to this region. */
+  std::list ins;
+  /*! \brief The outputs of this region */
+  std::list outs;
+  /*! \brief Nodes in this region. */
+  std::unordered_set nodes;
+
+  friend class AnnotatedRegionSet;
+  friend class AnnotatedRegionSetNode;
+};
+
+/*!
+ * \brief An object to hold the properties of a region as used by the
+ * AnnotatedRegionSet class. This should be considered read-only.
+*/
+class AnnotatedRegion : public ObjectRef {
+ public:
+  AnnotatedRegion() {
+auto n = make_object();
+data_ = std::move(n);
+  }
+
+  /*!
+ * \brief Construct from an object pointer.
+ * \param n The object pointer.
+ */
+  explicit AnnotatedRegion(ObjectPtr n) : ObjectRef(n) {}
+
+  /*! \return Mutable pointers to the node. */
+  AnnotatedRegionNode* operator->() const {
+auto* ptr = get_mutable();
+CHECK(ptr != nullptr);
+return static_cast(ptr);
+  }
+};
+
+class AnnotatedRegionSetNode : public Object {
+  using UnorderedRegionSet =
+  std::unordered_set;
+  // Create iterator alias for a RegionSet object.
+  using iterator = UnorderedRegionSet::iterator;
+  using const_iterator = UnorderedRegionSet::const_iterator;
+
+ public:
+  /*! \brief Default constructor. */
+  AnnotatedRegionSetNode() = default;
+
+  /*! \return The begin iterator */
+  iterator begin() {
+return regions_.begin();
+  }
+  /*! \return The end iterator */
+  iterator end() {
+return regions_.end();
+  }
+  /*! \return The const begin iterator */
+  const_iterator begin() const {
+return regions_.begin();
+  }
+  /*! \return The const end iterator */
+  const_iterator end() const {
+return regions_.end();
+  }
+
+  /*!
+   * \brief Get the region that an expression belongs to.
+   *
+   * \param expr Which expr to get the region for.
+   *
+   * \return A pointer to the region, nullptr if the expression
+   * doesn't belong to a region.
+   */
+  AnnotatedRegion GetRegion(const Expr& expr) const;
+
+  /*!
+ * \brief Merge region 1 into region 2.
+ *
+ * \param region1 A region to merge.
+ * \param region2 A region to merge.
+ */
+  void MergeRegions(AnnotatedRegion region1, AnnotatedRegion region2);
+
+  void VisitAttrs(AttrVisitor* v) {
+Array regions_array(regions_.begin(), regions_.end());
+v->Visit("regions", _array);
+  }
+
+  static constexpr const char* 

[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5030: [RELAY] Added a AnnotatedRegion utility class

2020-03-23 Thread GitBox
zhiics commented on a change in pull request #5030: [RELAY] Added a 
AnnotatedRegion utility class
URL: https://github.com/apache/incubator-tvm/pull/5030#discussion_r396652235
 
 

 ##
 File path: src/relay/analysis/annotated_region_set.cc
 ##
 @@ -0,0 +1,239 @@
+/*
+ * 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.
+ */
+
+#include "annotated_region_set.h"
+
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+
+namespace tvm {
+namespace relay {
+
+AnnotatedRegion AnnotatedRegionSetNode::GetRegion(const Expr& expr) const {
+  for (auto candidate : regions_) {
+if (candidate->nodes.find(expr) != candidate->nodes.end()) {
+  return candidate;
+}
+  }
+  return AnnotatedRegion(nullptr);
+}
+
+void AnnotatedRegionSetNode::MergeRegions(AnnotatedRegion region1,
+  AnnotatedRegion region2) {
+  if (region1 == region2) {
+return;
+  }
+
+  // Merge region 2 to region 1 and erase region 2.
+  region1->nodes.insert(region2->nodes.begin(), region2->nodes.end());
+  for (auto arg : region2->ins) {
+region1->ins.push_back(arg);
+  }
+  for (auto out : region2->outs) {
+region1->outs.push_back(out);
+  }
+  // if any of the outputs of 2 are inputs of 1, they become internal nodes
+  // so remove them from outs/args
+  std::vector args_to_remove;
+  for (const auto& arg : region1->ins) {
+auto call = Downcast(arg);
+auto it = std::find(region2->outs.begin(), region2->outs.end(), 
call->args[0]);
+if (it != region2->outs.end()) {
+  args_to_remove.push_back(arg);
+  region1->outs.remove(*it);
+}
+  }
+  for (const auto& arg : args_to_remove) {
+region1->ins.remove(arg);
+  }
+  regions_.erase(region2);
+}
+
+void AnnotatedRegionSetNode::AddToRegion(AnnotatedRegion region, const Expr& 
expr) {
+  auto region2 = GetRegion(expr);
+  if (region2.defined()) {
+MergeRegions(region, region2);
+  } else {
+region->nodes.insert(expr);
+  }
+}
+
+AnnotatedRegion AnnotatedRegionSetNode::MakeRegion() {
 
 Review comment:
   Likely we don't really need this helper. There is only one use of it. 
Inlining it to the caller with a comment clear enough.


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


With regards,
Apache Git Services


[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5030: [RELAY] Added a AnnotatedRegion utility class

2020-03-23 Thread GitBox
zhiics commented on a change in pull request #5030: [RELAY] Added a 
AnnotatedRegion utility class
URL: https://github.com/apache/incubator-tvm/pull/5030#discussion_r396639383
 
 

 ##
 File path: src/relay/analysis/annotated_region_set.h
 ##
 @@ -0,0 +1,277 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/relay/pass/annotated_region_set.h
+ * \brief Define data structures to extract and manipulate regions from
+ * a relay function. Regions are denoted by region_begin and region_end
+ * annotations that exist on all the input and output edges of the region.
+ */
+
+#ifndef TVM_RELAY_ANALYSIS_ANNOTATED_REGION_SET_H_
+#define TVM_RELAY_ANALYSIS_ANNOTATED_REGION_SET_H_
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace tvm {
+namespace relay {
+
+class AnnotatedRegion;
+class AnnotatedRegionSet;
+
+class AnnotatedRegionNode : public Object {
 
 Review comment:
   I would suggest we change this to a `struct` or just make members public 
since it only really just carries data. This getters seem to be redundant IMO.


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


With regards,
Apache Git Services


[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5030: [RELAY] Added a AnnotatedRegion utility class

2020-03-23 Thread GitBox
zhiics commented on a change in pull request #5030: [RELAY] Added a 
AnnotatedRegion utility class
URL: https://github.com/apache/incubator-tvm/pull/5030#discussion_r396643280
 
 

 ##
 File path: src/relay/analysis/annotated_region_set.h
 ##
 @@ -0,0 +1,277 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/relay/pass/annotated_region_set.h
+ * \brief Define data structures to extract and manipulate regions from
+ * a relay function. Regions are denoted by region_begin and region_end
+ * annotations that exist on all the input and output edges of the region.
+ */
+
+#ifndef TVM_RELAY_ANALYSIS_ANNOTATED_REGION_SET_H_
+#define TVM_RELAY_ANALYSIS_ANNOTATED_REGION_SET_H_
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace tvm {
+namespace relay {
+
+class AnnotatedRegion;
+class AnnotatedRegionSet;
+
+class AnnotatedRegionNode : public Object {
+ public:
+  void VisitAttrs(AttrVisitor* v) {
+v->Visit("id", );
+Array nodes_array(nodes.begin(), nodes.end());
+v->Visit("nodes", _array);
+Array args_array(ins.begin(), ins.end());
+v->Visit("args", _array);
+Array rets_array(outs.begin(), outs.end());
+v->Visit("rets", _array);
+  }
+
+  /*! \brief Get the region ID. */
+  int GetID() const {
+return id;
+  }
+
+  /*! \brief Get the region's inputs. */
+  std::list GetInputs() const {
+return ins;
+  }
+
+  /*! \brief Get the region's outputs. */
+  std::list GetOutputs() const {
+return outs;
+  }
+
+  /*! \brief Get the region's nodes. */
+  std::unordered_set GetNodes() const {
+return nodes;
+  }
+
+  static constexpr const char* _type_key = "relay.AnnotatedRegion";
+  TVM_DECLARE_FINAL_OBJECT_INFO(AnnotatedRegionNode, Object);
+
+ protected:
+  /*! \brief The region ID. */
+  int id{-1};
+  /*! \brief The inputs to this region. */
+  std::list ins;
+  /*! \brief The outputs of this region */
+  std::list outs;
+  /*! \brief Nodes in this region. */
+  std::unordered_set nodes;
+
+  friend class AnnotatedRegionSet;
+  friend class AnnotatedRegionSetNode;
+};
+
+/*!
+ * \brief An object to hold the properties of a region as used by the
+ * AnnotatedRegionSet class. This should be considered read-only.
+*/
+class AnnotatedRegion : public ObjectRef {
+ public:
+  AnnotatedRegion() {
+auto n = make_object();
+data_ = std::move(n);
+  }
+
+  /*!
+ * \brief Construct from an object pointer.
+ * \param n The object pointer.
+ */
+  explicit AnnotatedRegion(ObjectPtr n) : ObjectRef(n) {}
+
+  /*! \return Mutable pointers to the node. */
+  AnnotatedRegionNode* operator->() const {
+auto* ptr = get_mutable();
+CHECK(ptr != nullptr);
+return static_cast(ptr);
+  }
+};
+
+class AnnotatedRegionSetNode : public Object {
+  using UnorderedRegionSet =
+  std::unordered_set;
+  // Create iterator alias for a RegionSet object.
+  using iterator = UnorderedRegionSet::iterator;
+  using const_iterator = UnorderedRegionSet::const_iterator;
+
+ public:
+  /*! \brief Default constructor. */
+  AnnotatedRegionSetNode() = default;
+
+  /*! \return The begin iterator */
+  iterator begin() {
+return regions_.begin();
+  }
+  /*! \return The end iterator */
+  iterator end() {
+return regions_.end();
+  }
+  /*! \return The const begin iterator */
+  const_iterator begin() const {
+return regions_.begin();
+  }
+  /*! \return The const end iterator */
+  const_iterator end() const {
+return regions_.end();
+  }
+
+  /*!
+   * \brief Get the region that an expression belongs to.
+   *
+   * \param expr Which expr to get the region for.
+   *
+   * \return A pointer to the region, nullptr if the expression
+   * doesn't belong to a region.
+   */
+  AnnotatedRegion GetRegion(const Expr& expr) const;
+
+  /*!
+ * \brief Merge region 1 into region 2.
+ *
+ * \param region1 A region to merge.
+ * \param region2 A region to merge.
+ */
+  void MergeRegions(AnnotatedRegion region1, AnnotatedRegion region2);
 
 Review comment:
   use src and dest instead of region1 and region2. It seems the implementation 
merges region2 to region1


[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5030: [RELAY] Added a AnnotatedRegion utility class

2020-03-23 Thread GitBox
zhiics commented on a change in pull request #5030: [RELAY] Added a 
AnnotatedRegion utility class
URL: https://github.com/apache/incubator-tvm/pull/5030#discussion_r396641051
 
 

 ##
 File path: src/relay/analysis/annotated_region_set.h
 ##
 @@ -0,0 +1,277 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/relay/pass/annotated_region_set.h
+ * \brief Define data structures to extract and manipulate regions from
+ * a relay function. Regions are denoted by region_begin and region_end
+ * annotations that exist on all the input and output edges of the region.
+ */
+
+#ifndef TVM_RELAY_ANALYSIS_ANNOTATED_REGION_SET_H_
+#define TVM_RELAY_ANALYSIS_ANNOTATED_REGION_SET_H_
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace tvm {
+namespace relay {
+
+class AnnotatedRegion;
+class AnnotatedRegionSet;
+
+class AnnotatedRegionNode : public Object {
+ public:
+  void VisitAttrs(AttrVisitor* v) {
+v->Visit("id", );
+Array nodes_array(nodes.begin(), nodes.end());
+v->Visit("nodes", _array);
+Array args_array(ins.begin(), ins.end());
+v->Visit("args", _array);
+Array rets_array(outs.begin(), outs.end());
+v->Visit("rets", _array);
+  }
+
+  /*! \brief Get the region ID. */
+  int GetID() const {
+return id;
+  }
+
+  /*! \brief Get the region's inputs. */
+  std::list GetInputs() const {
+return ins;
+  }
+
+  /*! \brief Get the region's outputs. */
+  std::list GetOutputs() const {
+return outs;
+  }
+
+  /*! \brief Get the region's nodes. */
+  std::unordered_set GetNodes() const {
+return nodes;
+  }
+
+  static constexpr const char* _type_key = "relay.AnnotatedRegion";
+  TVM_DECLARE_FINAL_OBJECT_INFO(AnnotatedRegionNode, Object);
+
+ protected:
+  /*! \brief The region ID. */
+  int id{-1};
+  /*! \brief The inputs to this region. */
+  std::list ins;
+  /*! \brief The outputs of this region */
+  std::list outs;
+  /*! \brief Nodes in this region. */
+  std::unordered_set nodes;
+
+  friend class AnnotatedRegionSet;
+  friend class AnnotatedRegionSetNode;
+};
+
+/*!
+ * \brief An object to hold the properties of a region as used by the
+ * AnnotatedRegionSet class. This should be considered read-only.
+*/
+class AnnotatedRegion : public ObjectRef {
 
 Review comment:
   BTW, `AnnotatedRegion` is more just for internal use, right? Likely, we 
don't need to register them to the object system as they will not need to be 
exposed to Python.
   
   In this case, only need one class `AnnotatedRegion` to carry the data and 
`AnnotatedRegionNode` could be removed.


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


With regards,
Apache Git Services


[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5030: [RELAY] Added a AnnotatedRegion utility class

2020-03-23 Thread GitBox
zhiics commented on a change in pull request #5030: [RELAY] Added a 
AnnotatedRegion utility class
URL: https://github.com/apache/incubator-tvm/pull/5030#discussion_r396658432
 
 

 ##
 File path: python/tvm/relay/analysis/annotated_regions.py
 ##
 @@ -0,0 +1,62 @@
+# 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=no-else-return, unidiomatic-typecheck, invalid-name, 
unused-import
+"""Regions used in Relay."""
+
+from tvm.runtime import Object
+from . import _ffi_api
+
+
+class AnnotatedRegionSet(Object):
+"""Class to represent a relay expression split into regions."""
+
+def __init__(self, expr, region_begin_op, region_end_op):
 
 Review comment:
   We probably want to make compile_begin and end as default to the 
region_begin_op and region_end_op


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


With regards,
Apache Git Services


[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5030: [RELAY] Added a AnnotatedRegion utility class

2020-03-23 Thread GitBox
zhiics commented on a change in pull request #5030: [RELAY] Added a 
AnnotatedRegion utility class
URL: https://github.com/apache/incubator-tvm/pull/5030#discussion_r396636430
 
 

 ##
 File path: python/tvm/relay/analysis/annotated_regions.py
 ##
 @@ -0,0 +1,62 @@
+# 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=no-else-return, unidiomatic-typecheck, invalid-name, 
unused-import
+"""Regions used in Relay."""
+
+from tvm.runtime import Object
+from . import _ffi_api
+
+
+class AnnotatedRegionSet(Object):
+"""Class to represent a relay expression split into regions."""
+
+def __init__(self, expr, region_begin_op, region_end_op):
+"""Construct regions from an expression.
+
+Parameters
+--
+expr : tvm.relay.Expr
+The expression from which to construct the regions.
+region_begin_op : tvm.relay.Op
+The region begin annotation.
+region_end_op : tvm.relay.Op
+The region end annotation.
+
+"""
+self.__init_handle_by_constructor__(_ffi_api.AnnotatedRegionSet,
+expr,
+region_begin_op,
+region_end_op)
+
+def __len__(self):
+return len(self.regions)
+
+def get_region(self, expr):
+"""Get the region an expression belongs to.
+
+Parameters
+--
+expr : tvm.relay.Expr
+The expression.
+
+Returns
+---
+region : Region
 
 Review comment:
   Region isn't in Python, right?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5030: [RELAY] Added a AnnotatedRegion utility class

2020-03-23 Thread GitBox
zhiics commented on a change in pull request #5030: [RELAY] Added a 
AnnotatedRegion utility class
URL: https://github.com/apache/incubator-tvm/pull/5030#discussion_r396600188
 
 

 ##
 File path: python/tvm/relay/analysis/__init__.py
 ##
 @@ -19,9 +19,15 @@
 # Analysis passes
 from .analysis import *
 
+# Subgraphs
+from . import annotated_regions
+
 # Call graph
 from . import call_graph
 from .call_graph import CallGraph
 
 # Feature
 from . import feature
+
+CallGraph = call_graph.CallGraph
 
 Review comment:
   we've imported it above


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


With regards,
Apache Git Services


[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5030: [RELAY] Added a AnnotatedRegion utility class

2020-03-23 Thread GitBox
zhiics commented on a change in pull request #5030: [RELAY] Added a 
AnnotatedRegion utility class
URL: https://github.com/apache/incubator-tvm/pull/5030#discussion_r396644680
 
 

 ##
 File path: src/relay/analysis/annotated_region_set.h
 ##
 @@ -0,0 +1,277 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file tvm/relay/pass/annotated_region_set.h
+ * \brief Define data structures to extract and manipulate regions from
+ * a relay function. Regions are denoted by region_begin and region_end
+ * annotations that exist on all the input and output edges of the region.
+ */
+
+#ifndef TVM_RELAY_ANALYSIS_ANNOTATED_REGION_SET_H_
+#define TVM_RELAY_ANALYSIS_ANNOTATED_REGION_SET_H_
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace tvm {
+namespace relay {
+
+class AnnotatedRegion;
+class AnnotatedRegionSet;
+
+class AnnotatedRegionNode : public Object {
+ public:
+  void VisitAttrs(AttrVisitor* v) {
+v->Visit("id", );
+Array nodes_array(nodes.begin(), nodes.end());
+v->Visit("nodes", _array);
+Array args_array(ins.begin(), ins.end());
+v->Visit("args", _array);
+Array rets_array(outs.begin(), outs.end());
+v->Visit("rets", _array);
+  }
+
+  /*! \brief Get the region ID. */
+  int GetID() const {
+return id;
+  }
+
+  /*! \brief Get the region's inputs. */
+  std::list GetInputs() const {
+return ins;
+  }
+
+  /*! \brief Get the region's outputs. */
+  std::list GetOutputs() const {
+return outs;
+  }
+
+  /*! \brief Get the region's nodes. */
+  std::unordered_set GetNodes() const {
+return nodes;
+  }
+
+  static constexpr const char* _type_key = "relay.AnnotatedRegion";
+  TVM_DECLARE_FINAL_OBJECT_INFO(AnnotatedRegionNode, Object);
+
+ protected:
+  /*! \brief The region ID. */
+  int id{-1};
+  /*! \brief The inputs to this region. */
+  std::list ins;
+  /*! \brief The outputs of this region */
+  std::list outs;
+  /*! \brief Nodes in this region. */
+  std::unordered_set nodes;
+
+  friend class AnnotatedRegionSet;
+  friend class AnnotatedRegionSetNode;
+};
+
+/*!
+ * \brief An object to hold the properties of a region as used by the
+ * AnnotatedRegionSet class. This should be considered read-only.
+*/
+class AnnotatedRegion : public ObjectRef {
+ public:
+  AnnotatedRegion() {
+auto n = make_object();
+data_ = std::move(n);
+  }
+
+  /*!
+ * \brief Construct from an object pointer.
+ * \param n The object pointer.
+ */
+  explicit AnnotatedRegion(ObjectPtr n) : ObjectRef(n) {}
+
+  /*! \return Mutable pointers to the node. */
+  AnnotatedRegionNode* operator->() const {
+auto* ptr = get_mutable();
+CHECK(ptr != nullptr);
+return static_cast(ptr);
+  }
+};
+
+class AnnotatedRegionSetNode : public Object {
+  using UnorderedRegionSet =
+  std::unordered_set;
+  // Create iterator alias for a RegionSet object.
+  using iterator = UnorderedRegionSet::iterator;
+  using const_iterator = UnorderedRegionSet::const_iterator;
+
+ public:
+  /*! \brief Default constructor. */
+  AnnotatedRegionSetNode() = default;
+
+  /*! \return The begin iterator */
+  iterator begin() {
+return regions_.begin();
+  }
+  /*! \return The end iterator */
+  iterator end() {
+return regions_.end();
+  }
+  /*! \return The const begin iterator */
+  const_iterator begin() const {
+return regions_.begin();
+  }
+  /*! \return The const end iterator */
+  const_iterator end() const {
+return regions_.end();
+  }
+
+  /*!
+   * \brief Get the region that an expression belongs to.
+   *
+   * \param expr Which expr to get the region for.
+   *
+   * \return A pointer to the region, nullptr if the expression
+   * doesn't belong to a region.
+   */
+  AnnotatedRegion GetRegion(const Expr& expr) const;
 
 Review comment:
   As a utility, we might want to overload `operator[](const Expr& expr)`


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