[GitHub] [tvm] mbs-octoml commented on a change in pull request #9723: [Frontend] Add Span filling for frontends to Relay

[GitBox <gi...@apache.org>]

mbs-octoml commented on a change in pull request #9723:
URL: https://github.com/apache/tvm/pull/9723#discussion_r768889160



##########
File path: python/tvm/relay/frontend/common.py
##########
@@ -954,3 +955,53 @@ def try_resolve_var_to_const(x, graph_params):
         return _op.const(value, dtype)
 
     return x
+
+
+def set_span(sym, node_name):
+    """Set up the sapn for while converting OP"""
+
+    class SpanFiller(ExprMutator):
+        """SpanFiller"""
+
+        def __init__(self, node_name, surfix_str="_PART_"):

Review comment:
       nit: suffix_str

##########
File path: src/printer/relay_text_printer.cc
##########
@@ -389,12 +389,21 @@ Doc RelayTextPrinter::VisitExpr_(const TupleNode* op) {
   if (op->fields.size() == 1) {
     doc << ",";
   }
-  return doc << ")";
+  doc << ")";
+  if (op->span.defined()) {

Review comment:
       nit:: can you leave a warning comment that we'll probably need to protect this by some kind of 'include_spans' or 'verbose' printer flag. But at this stage I'm happy to have them all!

##########
File path: src/printer/relay_text_printer.cc
##########
@@ -389,12 +389,21 @@ Doc RelayTextPrinter::VisitExpr_(const TupleNode* op) {
   if (op->fields.size() == 1) {
     doc << ",";
   }
-  return doc << ")";
+  doc << ")";
+  if (op->span.defined()) {

Review comment:
       would you be up for doing the span suffix printing in the VisitExpr override? I think might as well do it for all the node types uniformly.




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