You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "kfeng123 (via GitHub)" <gi...@apache.org> on 2023/07/18 15:06:23 UTC

[GitHub] [tvm] kfeng123 opened a new pull request, #15350: [Bugfix] [Relay] fix a bug of printing dataflow pattern

kfeng123 opened a new pull request, #15350:
URL: https://github.com/apache/tvm/pull/15350

   When recursion of dataflow patterns is used, the pattern graph may not be a DAG. If recursion is encountered, ReprPrint of dataflow pattern may fall in a dead loop.
   This PR solves the bug of dataflow pattern printing, and is the first PR for the [pre-RFC](https://discuss.tvm.apache.org/t/on-the-applications-of-the-composition-of-dataflow-patterns/15344?u=kfeng123).
   


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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] masahi commented on pull request #15350: [Bugfix] [Relay] fix a bug of printing dataflow pattern

Posted by "masahi (via GitHub)" <gi...@apache.org>.
masahi commented on PR #15350:
URL: https://github.com/apache/tvm/pull/15350#issuecomment-1645233959

   btw, does the dataflow pattern support recursion at all? 


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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] kfeng123 commented on a diff in pull request #15350: [Bugfix] [Relay] fix a bug of printing dataflow pattern

Posted by "kfeng123 (via GitHub)" <gi...@apache.org>.
kfeng123 commented on code in PR #15350:
URL: https://github.com/apache/tvm/pull/15350#discussion_r1270501418


##########
include/tvm/relay/dataflow_pattern.h:
##########
@@ -537,6 +539,41 @@ DFPattern IsTuple(const Array<DFPattern>& fields);
 /*! \brief Syntatic Sugar for creating a TupleGetItemPattern*/
 DFPattern IsTupleGetItem(const DFPattern tuple, int index = -1);
 
+/*! \brief A printer class to print pattern. */
+class DFPatternPrinter : public ReprPrinter {
+ public:
+  std::stringstream string_stream{};
+
+  std::unordered_map<DFPattern, std::pair<size_t, std::string>, ObjectPtrHash, ObjectPtrEqual>
+      memo_{};
+  std::vector<DFPattern> recursed_patterns{};
+
+  DFPatternPrinter(std::ostream& stream)  // NOLINT(*)
+      : ReprPrinter(stream) {}
+  TVM_DLL void Print(const ObjectRef& node);
+  using FType = NodeFunctor<void(const ObjectRef&, DFPatternPrinter*)>;
+  TVM_DLL static FType& vtable();
+};
+
+inline std::ostream& operator<<(std::ostream& os,
+                                const DFPattern& n) {  // NOLINT(*)
+  std::stringstream string_stream{}, tmp_stream{};
+  DFPatternPrinter printer{tmp_stream};
+  printer.Print(n);
+  string_stream << "Main pattern is:" << std::endl;
+  string_stream << printer.string_stream.str();
+  string_stream << std::endl;
+  string_stream << "Auxiliary patterns are:";

Review Comment:
   Done



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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] kfeng123 commented on pull request #15350: [Bugfix] [Relay] fix a bug of printing dataflow pattern

Posted by "kfeng123 (via GitHub)" <gi...@apache.org>.
kfeng123 commented on PR #15350:
URL: https://github.com/apache/tvm/pull/15350#issuecomment-1646738494

   @tvm-bot rerun


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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] kfeng123 commented on pull request #15350: [Bugfix] [Relay] fix a bug of printing dataflow pattern

Posted by "kfeng123 (via GitHub)" <gi...@apache.org>.
kfeng123 commented on PR #15350:
URL: https://github.com/apache/tvm/pull/15350#issuecomment-1645258393

   > btw, does the dataflow pattern support recursion (loop) at all?
   
   Surprisingly, the answer is yes. Indeed, I was thinking to add the support for recursion, only to find that the existing dataflow pattern language has alreadly (silently) support it.
   But currently recursion may only implemented in C++, not python.
   The following is an example taken from the [pre-RFC](https://discuss.tvm.apache.org/t/on-the-applications-of-the-composition-of-dataflow-patterns/15344?u=kfeng123) doc:
   
   ``` python
   TVM_REGISTER_GLOBAL("relay.dataflow_pattern.my_pattern")
       .set_body_typed([]() {
       DFPattern dense_pattern = IsOp("nn.dense")({IsWildcard(), IsWildcard()});
       ObjectPtr<CallPatternNode> the_pattern_ptr = make_object<CallPatternNode>();
       the_pattern_ptr->op = IsOp("cast");
       the_pattern_ptr->args.clear();
       CallPattern the_pattern = CallPattern(the_pattern_ptr);
       AltPattern or_pattern{the_pattern, dense_pattern};
       the_pattern_ptr->args.push_back(or_pattern);
       //LOG(INFO) << PrettyPrint(the_pattern); // TODO: BUG!
     return the_pattern;
   });
   ```
   This simple pattern matches a nn.dense followed by an arbitrary number of cast. You can test this pattern via the following python code:
   
   ``` python
   class TheRewrite(DFPatternCallback):
       def __init__(self):
           super(TheRewrite, self).__init__(rewrite_once = True)
           pattern = tvm.get_global_func("relay.dataflow_pattern.my_pattern")()
           self.pattern = pattern
       def callback(self, pre, post, node_map):
           return relay.nn.relu(post)
   
   mod = create_model() # define a model
   the_rewrite = TheRewrite()
   out = rewrite(the_rewrite, mod["main"])
   ```
   
   Another application of recursion is PR [#15362](https://github.com/apache/tvm/pull/15362), which I do not know how to achieve without recursion. That PR is useful, and can really improve the computational graph for some quantized models.
   
   I would like to examine the pattern matching code further in the following days.
   If I find any bug with pattern recursion, I would like to raise PRs.


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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] kfeng123 commented on pull request #15350: [Bugfix] [Relay] fix a bug of printing dataflow pattern

Posted by "kfeng123 (via GitHub)" <gi...@apache.org>.
kfeng123 commented on PR #15350:
URL: https://github.com/apache/tvm/pull/15350#issuecomment-1640407339

   cc @masahi @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.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] tvm-bot commented on pull request #15350: [Bugfix] [Relay] fix a bug of printing dataflow pattern

Posted by "tvm-bot (via GitHub)" <gi...@apache.org>.
tvm-bot commented on PR #15350:
URL: https://github.com/apache/tvm/pull/15350#issuecomment-1640406135

   <!---bot-comment-->
   
   Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
   
   <!--bot-comment-ccs-start-->
    * cc @shingjan <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
   
   <sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>


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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] kfeng123 commented on a diff in pull request #15350: [Bugfix] [Relay] fix a bug of printing dataflow pattern

Posted by "kfeng123 (via GitHub)" <gi...@apache.org>.
kfeng123 commented on code in PR #15350:
URL: https://github.com/apache/tvm/pull/15350#discussion_r1270502402


##########
include/tvm/relay/dataflow_pattern.h:
##########
@@ -537,6 +539,41 @@ DFPattern IsTuple(const Array<DFPattern>& fields);
 /*! \brief Syntatic Sugar for creating a TupleGetItemPattern*/
 DFPattern IsTupleGetItem(const DFPattern tuple, int index = -1);
 
+/*! \brief A printer class to print pattern. */
+class DFPatternPrinter : public ReprPrinter {
+ public:
+  std::stringstream string_stream{};
+
+  std::unordered_map<DFPattern, std::pair<size_t, std::string>, ObjectPtrHash, ObjectPtrEqual>
+      memo_{};
+  std::vector<DFPattern> recursed_patterns{};

Review Comment:
   Documentation added. To be consistent with the printed text, I choose to name this variable to be auxiliary_patterns



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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] masahi merged pull request #15350: [Bugfix] [Relay] fix a bug of printing dataflow pattern

Posted by "masahi (via GitHub)" <gi...@apache.org>.
masahi merged PR #15350:
URL: https://github.com/apache/tvm/pull/15350


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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] masahi commented on a diff in pull request #15350: [Bugfix] [Relay] fix a bug of printing dataflow pattern

Posted by "masahi (via GitHub)" <gi...@apache.org>.
masahi commented on code in PR #15350:
URL: https://github.com/apache/tvm/pull/15350#discussion_r1270418150


##########
include/tvm/relay/dataflow_pattern.h:
##########
@@ -537,6 +539,41 @@ DFPattern IsTuple(const Array<DFPattern>& fields);
 /*! \brief Syntatic Sugar for creating a TupleGetItemPattern*/
 DFPattern IsTupleGetItem(const DFPattern tuple, int index = -1);
 
+/*! \brief A printer class to print pattern. */
+class DFPatternPrinter : public ReprPrinter {
+ public:
+  std::stringstream string_stream{};
+
+  std::unordered_map<DFPattern, std::pair<size_t, std::string>, ObjectPtrHash, ObjectPtrEqual>
+      memo_{};
+  std::vector<DFPattern> recursed_patterns{};
+
+  DFPatternPrinter(std::ostream& stream)  // NOLINT(*)
+      : ReprPrinter(stream) {}
+  TVM_DLL void Print(const ObjectRef& node);
+  using FType = NodeFunctor<void(const ObjectRef&, DFPatternPrinter*)>;
+  TVM_DLL static FType& vtable();
+};
+
+inline std::ostream& operator<<(std::ostream& os,
+                                const DFPattern& n) {  // NOLINT(*)
+  std::stringstream string_stream{}, tmp_stream{};
+  DFPatternPrinter printer{tmp_stream};
+  printer.Print(n);
+  string_stream << "Main pattern is:" << std::endl;
+  string_stream << printer.string_stream.str();
+  string_stream << std::endl;
+  string_stream << "Auxiliary patterns are:";

Review Comment:
   Remove "is" and "are" above



##########
include/tvm/relay/dataflow_pattern.h:
##########
@@ -537,6 +539,41 @@ DFPattern IsTuple(const Array<DFPattern>& fields);
 /*! \brief Syntatic Sugar for creating a TupleGetItemPattern*/
 DFPattern IsTupleGetItem(const DFPattern tuple, int index = -1);
 
+/*! \brief A printer class to print pattern. */
+class DFPatternPrinter : public ReprPrinter {
+ public:
+  std::stringstream string_stream{};
+
+  std::unordered_map<DFPattern, std::pair<size_t, std::string>, ObjectPtrHash, ObjectPtrEqual>
+      memo_{};
+  std::vector<DFPattern> recursed_patterns{};

Review Comment:
   Please document what you mean by "recursed_patterns". Probably "recursive_patterns" would be more correct.



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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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