You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/10/27 07:29:50 UTC

[GitHub] [tvm] sergey-grovety opened a new pull request, #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

sergey-grovety opened a new pull request, #13212:
URL: https://github.com/apache/tvm/pull/13212

   Added an option to tvmc and Ethos-U for printing to console or to the file which operators from the initial graph are offloaded to Ethos-U and which aren't. It forms line-by-line output of initial model IR, indicating which operations ported to Ethos-U.
   
   ### Usage
   ```
   # output to console:
   tvmc compile --target=ethos-u,cmsis-nn,c \
       --target-ethos-u-dump_npu_functions_coverage=- \
       ........
   
   # output to file:
   tvmc compile --target=ethos-u,cmsis-nn,c \
       --target-ethos-u-dump_npu_functions_coverage=<file path> \
       ........
   ```
   
   ### Example output:
   ```
   ...
   ethos-u    <-   %1 = nn.bias_add(%0, %v_param_2, axis=3);
   ethos-u    <-   %2 = qnn.requantize(%1, meta[relay.Constant][1], 0, 0.0235294f, -128, axis=3, out_dtype="int8");
   ethos-u    <-   %3 = clip(%2, a_min=-128f, a_max=127f);
   ....
   ```


-- 
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 #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13212:
URL: https://github.com/apache/tvm/pull/13212#issuecomment-1293109146

   <!---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 @Mousius, @gromero, @lhutton1 <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] arina-grovety commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by "arina-grovety (via GitHub)" <gi...@apache.org>.
arina-grovety commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1098760968


##########
python/tvm/relay/frontend/common.py:
##########
@@ -1067,6 +1067,20 @@ def __init__(self, span):
             self._span = tvm.relay.Span(tvm.relay.SourceName(span.decode("utf-8")), 0, 0, 0, 0)
         else:
             assert False, f"unsupported span type: {type(span)}"
+        self.suffix_str = "_PART_"
+        self.counter = 0
+        self.distance_from_leaf = -1
+
+    def _create_span(self):
+        """Adds suffix_str + counter value to _span.source_name.name,
+        to create a unique source_name for the Relay layer
+        """
+        if self.distance_from_leaf == 0:
+            return tvm.relay.Span(tvm.relay.SourceName(self._span), 0, 0, 0, 0)
+        self.distance_from_leaf -= 1
+        span_str = "{}{}{}".format(self._span.source_name.name, self.suffix_str, str(self.counter))
+        self.counter += 1
+        return tvm.relay.Span(tvm.relay.SourceName(span_str), 0, 0, 0, 0)

Review Comment:
   
   > Because no rewriting/mutation is invoked in `dump_operation_offloads()`, hash value won't be changed. Maybe hash value is an acceptable alternative?
   > 
   
   Hello @chunit-quic, 
   I apologize for the delay in replying. Thank you very much for your explanations about suffixes! 
   Got it, I'll try to use hash 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.

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

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


[GitHub] [tvm] lhutton1 commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1039660864


##########
python/tvm/relay/frontend/common.py:
##########
@@ -997,3 +998,57 @@ def try_resolve_var_to_const(x, graph_params):
         return _op.const(value, dtype)
 
     return x
+
+
+# Span filling to Relay for TFLite taken from
+# commit https://github.com/apache/tvm/pull/9723

Review Comment:
   Thanks, yep I think that would be best, it seems like the review is almost finished in that PR 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.

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

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


[GitHub] [tvm] ashutosh-arm commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by "ashutosh-arm (via GitHub)" <gi...@apache.org>.
ashutosh-arm commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1088919482


##########
python/tvm/driver/tvmc/compiler.py:
##########
@@ -459,3 +493,65 @@ def save_dumps(module_name: str, dumps: Dict[str, str], dump_root: str = "."):
         dump_name = module_name + "." + dump_format
         with open(Path(dump_root, dump_name), "w") as f:
             f.write(dumps[dump_format])
+
+
+def dump_operation_offloads(mod: tvm.ir.IRModule, initial_relay_astext: list, dump_path: str):
+    """This helper function forms a line-by-line output of the initial Relay lines,
+    indicating which operations are ported to which backend,
+    indicating the composite that includes those operations e.g
+    'device1    <- device2.qnn_conv2d'
+    'device1    <-        %0 = qnn.conv2d(%tfl.quantize, %v_param_1, ...'
+    'device1    <-        %1 = nn.bias_add(%0, %v_param_2, axis=3);'
+    'device1    <-        %2 = qnn.requantize(%1, meta[relay.Constant]...'
+    'device2    <- device2.reshape'
+    'device2    <-        %3 = reshape(%206, newshape=[1, 1001]);'
+
+    Parameters
+    ----------
+    mod : tvm.ir.IRModule
+        The IRModule that gets generated from a relay frontend.
+    initial_relay_astext : list

Review Comment:
   I would suggest the same thing as @lhutton1 did above. Text representation changes quite often. It is better to rely on the information available inside the module object and extract it using let's say `ExprVisitor`.  



-- 
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] arina-grovety commented on pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
arina-grovety commented on PR #13212:
URL: https://github.com/apache/tvm/pull/13212#issuecomment-1310432560

   
   > 4. Another reason for thinking twice is that this knob changes the output from TVM. We should seek more opinions from community maybe?
   
   Can you please clarify what do you mean by "the output from 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.

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

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


[GitHub] [tvm] arina-grovety commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by "arina-grovety (via GitHub)" <gi...@apache.org>.
arina-grovety commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1108215937


##########
python/tvm/relay/frontend/common.py:
##########
@@ -1067,6 +1067,20 @@ def __init__(self, span):
             self._span = tvm.relay.Span(tvm.relay.SourceName(span.decode("utf-8")), 0, 0, 0, 0)
         else:
             assert False, f"unsupported span type: {type(span)}"
+        self.suffix_str = "_PART_"
+        self.counter = 0
+        self.distance_from_leaf = -1
+
+    def _create_span(self):
+        """Adds suffix_str + counter value to _span.source_name.name,
+        to create a unique source_name for the Relay layer
+        """
+        if self.distance_from_leaf == 0:
+            return tvm.relay.Span(tvm.relay.SourceName(self._span), 0, 0, 0, 0)
+        self.distance_from_leaf -= 1
+        span_str = "{}{}{}".format(self._span.source_name.name, self.suffix_str, str(self.counter))
+        self.counter += 1
+        return tvm.relay.Span(tvm.relay.SourceName(span_str), 0, 0, 0, 0)

Review Comment:
   Hello @lhutton1 , @chunit-quic 
   
   We would propose to accept changes for this PR despite the fact that it is based on the use of suffixes, which in the future will be abolished. The point is that there is also a TVMC option implemented here with the output of which initial Relay operations to which target are offloaded. When in the future the suffix mechanism will be replaced by some other one, we can replace its use for this TVMC option.



-- 
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] arina-grovety commented on pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

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

   @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] ashutosh-arm commented on pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on PR #13212:
URL: https://github.com/apache/tvm/pull/13212#issuecomment-1300013307

   I agree with @lhutton1 here. The knob `--dump-code="relay"` provides a way to visualize the post-partition relay model. `main` function in this relay model lists sequence of calls to partitioned functions with appropriate target annotations. Does the new knob `print_operators_offloading` serve any additional purpose that I might have missed @sergey-grovety ?


-- 
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] arina-grovety commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
arina-grovety commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1016380651


##########
src/relay/backend/contrib/ethosu/compiler_attrs.cc:
##########
@@ -56,6 +57,11 @@ struct EthosUCompilerConfigNode : public tvm::AttrsNode<EthosUCompilerConfigNode
             "The class of Arm(R) Ethos(TM)-U NPU; possible values = {ethos-u55-32, ethos-u55-64, "
             "ethos-u55-128, ethos-u55-256}")
         .set_default("ethos-u55-256");
+    TVM_ATTR_FIELD(dump_npu_functions_coverage)
+        .describe(
+            "Whether info about ethosu partition for input module layers should be dumped; "
+            "possible values for dump to console / file, respectively = {-, <path_to_file>}")
+        .set_default("");

Review Comment:
   Thanks @lhutton1, I agree with most of the suggestions.
   Concerning the default value, there is a concern that if we write output to the console by default, wouldn't that clutter up the output? Seems like the ability to not get it if it's not needed would be handy.



-- 
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] lhutton1 commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1010579071


##########
src/relay/backend/contrib/ethosu/compiler_attrs.cc:
##########
@@ -56,6 +57,11 @@ struct EthosUCompilerConfigNode : public tvm::AttrsNode<EthosUCompilerConfigNode
             "The class of Arm(R) Ethos(TM)-U NPU; possible values = {ethos-u55-32, ethos-u55-64, "
             "ethos-u55-128, ethos-u55-256}")
         .set_default("ethos-u55-256");
+    TVM_ATTR_FIELD(dump_npu_functions_coverage)
+        .describe(
+            "Whether info about ethosu partition for input module layers should be dumped; "
+            "possible values for dump to console / file, respectively = {-, <path_to_file>}")
+        .set_default("");

Review Comment:
   I think the default should be to output to console ("-")



-- 
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] lhutton1 commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1016387185


##########
src/relay/backend/contrib/ethosu/compiler_attrs.cc:
##########
@@ -56,6 +57,11 @@ struct EthosUCompilerConfigNode : public tvm::AttrsNode<EthosUCompilerConfigNode
             "The class of Arm(R) Ethos(TM)-U NPU; possible values = {ethos-u55-32, ethos-u55-64, "
             "ethos-u55-128, ethos-u55-256}")
         .set_default("ethos-u55-256");
+    TVM_ATTR_FIELD(dump_npu_functions_coverage)
+        .describe(
+            "Whether info about ethosu partition for input module layers should be dumped; "
+            "possible values for dump to console / file, respectively = {-, <path_to_file>}")
+        .set_default("");

Review Comment:
   Oops, yep you're correct, apologies!



-- 
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] ashutosh-arm commented on pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on PR #13212:
URL: https://github.com/apache/tvm/pull/13212#issuecomment-1310068371

   
   > 
   > Hi @ashutosh-arm sorry for my late reply. As I see it, the main purpose of the new option is to show the correspondence between the operators from the original graph and the final operations offloading on the target. This is displayed as a sequential printout of the source relay's operations, with the composites from which they are derived and the target to which they are unloaded.
   > 
   > Another point worth highlighting is the partitioned Relay, which is an output of --dump-code="relay", have relay operation's numbers ( %...) different from those in the initial Relay. Therefore, the new knob, which keeps the initial Relay's numbers, can be handy
   > 
   > Here is an example output with the new option:
   > 
   > ```
   > 'ethos-u    <- ethos-u.qnn_conv2d'
   
   I see your point. Here are some points to consider:
   
   1. `print_operators_offloading` would print the mapping of MicroNPU operators to the original Relay operators. Each target that uses partitioning has its own ways of defining these mappings. Some of them make use of `MergeCompilerRegions` that clubs multiple operators into a single partitioned function. Would it be possible to support additional targets given the knob name implies generic support in TVM?
   2. Another thing in pipe for better debug from frontends down to TIR is Compiler Explorer: https://github.com/apache/tvm/issues/13116. At some point in future, this should provide more transparency to the operator mappings.
   3. In the mean time, with a little education to the end user of MicroNPU, learning to interpret partitioned functions could be made easy. Just a suggestion, this knowledge could be shared via [TVM docs](https://tvm.apache.org/docs/how_to/work_with_microtvm/micro_ethosu.html?highlight=ethosu)  and/or [MicroNPU demo](https://github.com/apache/tvm/blob/main/apps/microtvm/ethosu/README.md.).
   4. Another reason for thinking twice is that this knob changes the output from TVM. We should seek more opinions from community maybe?


-- 
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] arina-grovety commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
arina-grovety commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1016380651


##########
src/relay/backend/contrib/ethosu/compiler_attrs.cc:
##########
@@ -56,6 +57,11 @@ struct EthosUCompilerConfigNode : public tvm::AttrsNode<EthosUCompilerConfigNode
             "The class of Arm(R) Ethos(TM)-U NPU; possible values = {ethos-u55-32, ethos-u55-64, "
             "ethos-u55-128, ethos-u55-256}")
         .set_default("ethos-u55-256");
+    TVM_ATTR_FIELD(dump_npu_functions_coverage)
+        .describe(
+            "Whether info about ethosu partition for input module layers should be dumped; "
+            "possible values for dump to console / file, respectively = {-, <path_to_file>}")
+        .set_default("");

Review Comment:
   Thanks @lhutton1, I agree with most of the suggestions.
   As for the default value, there is a concern that if we write output to the console by default, wouldn't that clutter up the output? Seems like the ability to not get it if it's not needed would be handy.



-- 
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] sergey-grovety commented on pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
sergey-grovety commented on PR #13212:
URL: https://github.com/apache/tvm/pull/13212#issuecomment-1297001688

   @leandron could you please check 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.

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

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


[GitHub] [tvm] lhutton1 commented on pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on PR #13212:
URL: https://github.com/apache/tvm/pull/13212#issuecomment-1330982489

   Thanks for the updates @sergey-grovety @arina-grovety, looks great! I started reviewing this evening but didn't fully get through it, will pick up where I left off tomorrow


-- 
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] lhutton1 commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1039686396


##########
python/tvm/driver/tvmc/compiler.py:
##########
@@ -459,3 +493,65 @@ def save_dumps(module_name: str, dumps: Dict[str, str], dump_root: str = "."):
         dump_name = module_name + "." + dump_format
         with open(Path(dump_root, dump_name), "w") as f:
             f.write(dumps[dump_format])
+
+
+def dump_operation_offloads(mod: tvm.ir.IRModule, initial_relay_astext: list, dump_path: str):
+    """This helper function forms a line-by-line output of the initial Relay lines,
+    indicating which operations are ported to which backend,
+    indicating the composite that includes those operations e.g
+    'device1    <- device2.qnn_conv2d'
+    'device1    <-        %0 = qnn.conv2d(%tfl.quantize, %v_param_1, ...'
+    'device1    <-        %1 = nn.bias_add(%0, %v_param_2, axis=3);'
+    'device1    <-        %2 = qnn.requantize(%1, meta[relay.Constant]...'
+    'device2    <- device2.reshape'
+    'device2    <-        %3 = reshape(%206, newshape=[1, 1001]);'
+
+    Parameters
+    ----------
+    mod : tvm.ir.IRModule
+        The IRModule that gets generated from a relay frontend.
+    initial_relay_astext : list

Review Comment:
   I was thinking that traversing the Relay IR itself here might simplify the logic below and decouple the implementation from the textual representation making it more robust to changes in the future. It seems like it would also remove the need to make changes such as https://github.com/apache/tvm/pull/13212/files#diff-237c52e4e68362990738b47cc97c81b5c84ec92dfbcb672e961f0e9887f436c0R378 which might require more motivation from the community. WDYT?
   
   cc @ashutosh-arm @ekalda incase you have any other suggestions



-- 
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] lhutton1 commented on pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on PR #13212:
URL: https://github.com/apache/tvm/pull/13212#issuecomment-1312097771

   > > The option for printing the operators currently seems very specific to the NPU, I'm wondering if we would see more benefit adding this as a generic option within TVMC without too many changes? Not only would it benefit other targets, it would make the option more robust and easier to find from a user POV. Its currently possible to save the partitioned graph in TVMC using `--dump-code="relay"`, perhaps `print_operators_offloading` could be called at a similar point (given a command line argument such as `--dump-offloads`) rather than from within the NPU specific code, WDYT?
   > 
   > Hi @lhutton1,
   > 
   > Do you propose to implement this function for all the targets? Or just add a general compiler option leaving the implementation currently only in the ethos-u backend? Right now, this function is specific to ethos-u and is handled in the ethos-u backend. As far as I'm concerned it won't be a problem to implement the function for all targets, but of course I could be wrong.
   > 
   > Here is an example how the output would look like if model is compiled for the target "llvm":
   > 
   > ```
   >    'generic    <-   %0 = qnn.conv2d(%tfl.quantize, %v_param_1, ...'
   >    'generic    <-   %1 = nn.bias_add(%0, %v_param_2, axis=3);'
   >    'generic    <-   %2 = qnn.requantize(%1, meta[relay.Constant]...'
   > ```
   > 
   > And for targets "ethos-u,cmsis-nn,c"
   > 
   > ```
   >     'ethos-u    <- ethos-u.qnn_conv2d'
   >     'ethos-u    <-        %204 = qnn.conv2d(%203, %v_param_105, -128, 0, 0.0235294f, ...'
   >     'ethos-u    <-        %205 = nn.bias_add(%204, %v_param_106, axis=3);'
   >     'ethos-u    <-        %206 = qnn.requantize(%205, meta[relay.Constant][105], 0, ...'
   >     'ethos-u    <- ethos-u.reshape'
   >     'ethos-u    <-        %207 = reshape(%206, newshape=[1, 1001]);'
   >     'cmsis-nn   <- cmsis-nn.qnn_softmax'
   >     'cmsis-nn   <-        %208 = qnn.dequantize(%207, 0.0775722f, -61);'
   >     'cmsis-nn   <-        %209 = nn.softmax(%208);'
   >     'cmsis-nn   <-        qnn.quantize(%209, 0.00390625f, -128, out_dtype="int8")'
   > ```
   
   Thanks for the explanation @arina-grovety, yes I was thinking other backends like CMSIS-NN could make use of the same approach since the `AnalyzeOperationsDistribution` already seems quite generic. Where possible we can add the composite function names and if they are not found we can fallback to just printing the Relay - exactly as you described. If another backend has a different method of offloading operations this could simply be added to the pass in the future as and when needed.
   
   I see @ashutosh-arm's point that this feature intersects with the work in https://github.com/apache/tvm/issues/13116, perhaps it would be useful to have a discussion with the authors to align on expectations. In the future it would be great to see the `ethos-u.qnn_conv2d` annotations relate back to the input graph format (e.g. in TFLite: CONV2D) to make it easy for the user to relate their compiled operations to their original graph, but this seems a bit involved for now.
   
   > 1. `print_operators_offloading` would print the mapping of MicroNPU operators to the original Relay operators. Each target that uses partitioning has its own ways of defining these mappings. Some of them make use of `MergeCompilerRegions` that clubs multiple operators into a single partitioned function. Would it be possible to support additional targets given the knob name implies generic support in TVM?
   
   @ashutosh-arm, I think this is okay as operators will still be wrapped in their respective 'composite' function where the "ethos-u.qnn_conv2d" name is stored. Supporting other methods of operator offloading such as https://github.com/apache/tvm/blob/main/python/tvm/relay/op/contrib/ethosn.py#L451 I feel are out of scope for this work for the time being. I agree that we should get the community opinion on this.


-- 
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] ashutosh-arm commented on pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on PR #13212:
URL: https://github.com/apache/tvm/pull/13212#issuecomment-1311647287

   
   > Can you please clarify what do you mean by "the output from TVM"?
   Sorry for wrong wording. With this knob, TVM will produce a new debug output which would set an example for other backends. So, my suggestion was to discuss this upfront on the discuss forum.
   


-- 
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] chunit-quic commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by "chunit-quic (via GitHub)" <gi...@apache.org>.
chunit-quic commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1090188874


##########
python/tvm/relay/frontend/common.py:
##########
@@ -1067,6 +1067,20 @@ def __init__(self, span):
             self._span = tvm.relay.Span(tvm.relay.SourceName(span.decode("utf-8")), 0, 0, 0, 0)
         else:
             assert False, f"unsupported span type: {type(span)}"
+        self.suffix_str = "_PART_"
+        self.counter = 0
+        self.distance_from_leaf = -1
+
+    def _create_span(self):
+        """Adds suffix_str + counter value to _span.source_name.name,
+        to create a unique source_name for the Relay layer
+        """
+        if self.distance_from_leaf == 0:
+            return tvm.relay.Span(tvm.relay.SourceName(self._span), 0, 0, 0, 0)
+        self.distance_from_leaf -= 1
+        span_str = "{}{}{}".format(self._span.source_name.name, self.suffix_str, str(self.counter))
+        self.counter += 1
+        return tvm.relay.Span(tvm.relay.SourceName(span_str), 0, 0, 0, 0)

Review Comment:
   > This seems sensible to me, cc the original author of span filling @chunit-quic just to check
   
   Hi all,
   
   Thank you for informing us this change, we just check the PR. We have some ideas about it, and we would like to share some discussions about the suffix in the forum with you.
   
   About the way how span is used in the PR, it looks to me like you would like to take the
   source_name of span as an indicator to the original Relay call(expression). 
   We find the hash value might be enough for your use case rather than making a unique source_name for a span. Please allow me to describe some problems might be cuased by adding suffix.
   
   We removed suffix in the new implementation in our [preRFC](https://discuss.tvm.apache.org/t/pre-rfc-tvm-explorer-infrastructure/13457#reference-level-explanation-9). In short, we found adding suffix might mislead in some cases. Here is an example:
   
   Suppose:
   1. An user thinks span meaning where an expression is from
   2. A conv2d layer is converted to 2 relay exprs (conv2d and relu) in a specific frontend.
   
   The conversion of a simple frontend model with only a conv2d layer will result in this:
   
   ```python
   #FRONTEND
   conv2d layer, named as my_conv2d
   #Relay IR
   %0 = conv2d() /* my_conv2d_part_0 */
   %1 = relu() /* my_conv2d_part_1 */
   ```
   In the case above, that user might assume there are "two layers" in the original frontend model, rather than thinking the converted Relay exprs are from the same forntend layer.
   One is conv2d layer with the name "my_conv2d_part_0", and a relu layer which is named as
   "my_conv2d_part_1".
   
   Yet with help of suffix we do have some advantages. Like allocating the expression which generates the tensor output result of its frontend layer. So far we have [many discussions](https://discuss.tvm.apache.org/t/pre-rfc-tvm-explorer-infrastructure/13457/26?u=chunit)
   with community but we are still thinking about whether there is a better way to deal with.
   
   Therefore currently we won't recommend you to add suffix to the source_name of span. After checking the codes, perhaps you could consider using the hash value, `tvm.ir.structural_hash()` of an expr as the key of dictionary, like:
   
   ```python
   #python/tvm/relay/analysis/operations_distribution.py
   class AnalyzeOperationsDistribution(ExprVisitor):
       #...
       def visit_call(self, call: relay.Call):
           if isinstance(call.op, tvm.ir.Op):
               self.unique_op_ids[tvm.ir.structural_hash(call)] = [self.compiler_name, self.func_name, self.func_id]
   #python/tvm/driver/tvmc/compiler.py
   def dump_operation_offloads(...):
       def annotate_f(x):
           ret = ""
           if isinstance(x, relay.Call):
               compiler_name, op_name, func_id = operations_distribution[tvm.ir.structural_hash(x)]
               ret = f", func_id: {func_id}, compiler_name: {compiler_name}, op_name: {op_name}"
           return ret
   ```
   
   Because no rewriting/mutation is invoked in `dump_operation_offloads()`, hash value won't be changed. Maybe hash value is an acceptable alternative?
   
   Thank you for reading! Feel free if you have any question. We will try to get back to you soon. :)
   
   



-- 
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] arina-grovety commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by "arina-grovety (via GitHub)" <gi...@apache.org>.
arina-grovety commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1114164082


##########
python/tvm/relay/frontend/common.py:
##########
@@ -1067,6 +1067,20 @@ def __init__(self, span):
             self._span = tvm.relay.Span(tvm.relay.SourceName(span.decode("utf-8")), 0, 0, 0, 0)
         else:
             assert False, f"unsupported span type: {type(span)}"
+        self.suffix_str = "_PART_"
+        self.counter = 0
+        self.distance_from_leaf = -1
+
+    def _create_span(self):
+        """Adds suffix_str + counter value to _span.source_name.name,
+        to create a unique source_name for the Relay layer
+        """
+        if self.distance_from_leaf == 0:
+            return tvm.relay.Span(tvm.relay.SourceName(self._span), 0, 0, 0, 0)
+        self.distance_from_leaf -= 1
+        span_str = "{}{}{}".format(self._span.source_name.name, self.suffix_str, str(self.counter))
+        self.counter += 1
+        return tvm.relay.Span(tvm.relay.SourceName(span_str), 0, 0, 0, 0)

Review Comment:
   Hello @chunit-quic,
   thank you for the detailed explanations!
   
   I'm sorry, but I'm a bit confused. Let me explain my question:
   
   First, do I understand correctly that the workflow in my case will be as follows:
   
   frontend model -> Relay IR (IR1)
   if --dump-offloads option is on
   &nbsp;&nbsp;  tag IR1 with suffixes using the _SuffixTagger pass
   inside the tvmc.compiler.compile_model:
   &nbsp;&nbsp;&nbsp;  ...
   &nbsp;&nbsp;&nbsp;  IR1(optionally tagged) transfoms by partitions for selected backends (IR2)
   &nbsp;&nbsp;&nbsp;  if --dump-offloads option is on
   &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;  dump initial_relay offloaded result
   &nbsp;&nbsp;&nbsp;  ... 
           
   And here is my question:
   If I understand correctly, the suffix, although optional, sometimes will still be present in the Relay.
   Doesn't this mean that, although optional, we will again get the situation we wanted to avoid by removing the suffixes?



-- 
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] arina-grovety commented on pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
arina-grovety commented on PR #13212:
URL: https://github.com/apache/tvm/pull/13212#issuecomment-1308761777

   > I agree with @lhutton1 here. The knob `--dump-code="relay"` provides a way to visualize the post-partition relay model. `main` function in this relay model lists sequence of calls to partitioned functions with appropriate target annotations. Does the new knob `print_operators_offloading` serve any additional purpose that I might have missed @sergey-grovety ? To be fair, I have only read the PR description 😅
   
   Hi @ashutosh-arm sorry for my late reply. As I see it, the main purpose of the new option is to show the correspondence between the operators from the original graph and the final operations offloading on the target. This is displayed as a sequential printout of the source relay's operations, with the composites from which they are derived and the target to which they are unloaded.
   
   Another point worth highlighting is the partitioned Relay, which is an output of --dump-code="relay", have relay operation's numbers ( %...) different from those in the initial Relay. Therefore, the new knob, which keeps the initial Relay's numbers, can be handy
   
   Here is an example output with the new option:
   
       'ethos-u    <- ethos-u.qnn_conv2d'
       'ethos-u    <-        %204 = qnn.conv2d(%203, %v_param_105, -128, 0, 0.0235294f, ...'
       'ethos-u    <-        %205 = nn.bias_add(%204, %v_param_106, axis=3);'
       'ethos-u    <-        %206 = qnn.requantize(%205, meta[relay.Constant][105], 0, ...'
       'ethos-u    <- ethos-u.reshape'
       'ethos-u    <-        %207 = reshape(%206, newshape=[1, 1001]);'
       
   


-- 
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] arina-grovety commented on pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
arina-grovety commented on PR #13212:
URL: https://github.com/apache/tvm/pull/13212#issuecomment-1308844686

   
   > 
   > The option for printing the operators currently seems very specific to the NPU, I'm wondering if we would see more benefit adding this as a generic option within TVMC without too many changes? Not only would it benefit other targets, it would make the option more robust and easier to find from a user POV. Its currently possible to save the partitioned graph in TVMC using `--dump-code="relay"`, perhaps `print_operators_offloading` could be called at a similar point (given a command line argument such as `--dump-offloads`) rather than from within the NPU specific code, WDYT?
   > 
   
   Hi @lhutton1, 
   
   Do you propose to implement this function for all the targets? Or just add a general compiler option leaving the implementation currently only in the ethos-u backend? 
   Right now, this function is specific to ethos-u and is handled in the ethos-u backend.
   As far as I'm concerned it won't be a problem to implement the function for all targets, but of course I could be wrong.
   
   Here is an example how the output would look like if model is compiled for the target "llvm":
      'generic    <-   %0 = qnn.conv2d(%tfl.quantize, %v_param_1, ...'
      'generic    <-   %1 = nn.bias_add(%0, %v_param_2, axis=3);'
      'generic    <-   %2 = qnn.requantize(%1, meta[relay.Constant]...'
   
   And for targets "ethos-u,cmsis-nn,c"
       'ethos-u    <- ethos-u.qnn_conv2d'
       'ethos-u    <-        %204 = qnn.conv2d(%203, %v_param_105, -128, 0, 0.0235294f, ...'
       'ethos-u    <-        %205 = nn.bias_add(%204, %v_param_106, axis=3);'
       'ethos-u    <-        %206 = qnn.requantize(%205, meta[relay.Constant][105], 0, ...'
       'ethos-u    <- ethos-u.reshape'
       'ethos-u    <-        %207 = reshape(%206, newshape=[1, 1001]);'
       'cmsis-nn   <- cmsis-nn.qnn_softmax'
       'cmsis-nn   <-        %208 = qnn.dequantize(%207, 0.0775722f, -61);'
       'cmsis-nn   <-        %209 = nn.softmax(%208);'
       'cmsis-nn   <-        qnn.quantize(%209, 0.00390625f, -128, out_dtype="int8")'


-- 
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] lhutton1 commented on pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

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

   @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] arina-grovety commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by "arina-grovety (via GitHub)" <gi...@apache.org>.
arina-grovety commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1114164082


##########
python/tvm/relay/frontend/common.py:
##########
@@ -1067,6 +1067,20 @@ def __init__(self, span):
             self._span = tvm.relay.Span(tvm.relay.SourceName(span.decode("utf-8")), 0, 0, 0, 0)
         else:
             assert False, f"unsupported span type: {type(span)}"
+        self.suffix_str = "_PART_"
+        self.counter = 0
+        self.distance_from_leaf = -1
+
+    def _create_span(self):
+        """Adds suffix_str + counter value to _span.source_name.name,
+        to create a unique source_name for the Relay layer
+        """
+        if self.distance_from_leaf == 0:
+            return tvm.relay.Span(tvm.relay.SourceName(self._span), 0, 0, 0, 0)
+        self.distance_from_leaf -= 1
+        span_str = "{}{}{}".format(self._span.source_name.name, self.suffix_str, str(self.counter))
+        self.counter += 1
+        return tvm.relay.Span(tvm.relay.SourceName(span_str), 0, 0, 0, 0)

Review Comment:
   Hello @chunit-quic,
   thank you for the detailed explanations!
   
   I'm sorry, but I'm a bit confused. Let me explain my question:
   
   First, do I understand correctly that the workflow in my case will be as follows:
   
   ```
   frontend model -> Relay IR (IR1)
   if --dump-offloads option is on
        tag IR1 with suffixes using the _SuffixTagger pass
   inside the tvmc.compiler.compile_model:
        ...
        IR1(optionally tagged) transfoms by partitions for selected backends (IR2)
        if --dump-offloads option is on
             dump initial_relay offloaded result
        ... 
   ```
           
   And here is my question:
   If I understand correctly, the suffix, although optional, sometimes will still be present in the Relay.
   Doesn't this mean that, although optional, we will again get the situation we wanted to avoid by removing the suffixes?



-- 
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] lhutton1 commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1035009043


##########
python/tvm/driver/tvmc/compiler.py:
##########
@@ -257,6 +274,10 @@ def compile_model(
     dump_code : list, optional
         Dump the generated code for the specified source types, on
         the requested target.
+    dump_offloads : str
+        Dump the the information about the partition of input model's layers by external codegen.

Review Comment:
   nit: s/the the/the



##########
python/tvm/relay/analysis/operations_distribution.py:
##########
@@ -0,0 +1,86 @@
+# 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.
+"""Utilities that enable analyze Relay and get mappings for unique
+input module layer name to the tuple of compiler and operation name"""
+import tvm
+from tvm import relay
+from tvm.relay.expr_functor import ExprVisitor
+
+
+class AnalyzeOperationsDistribution(ExprVisitor):
+    """A visitor pass that maintai the dictionary unic_op_ids where

Review Comment:
   nit: maintains



##########
python/tvm/driver/tvmc/compiler.py:
##########
@@ -459,3 +493,65 @@ def save_dumps(module_name: str, dumps: Dict[str, str], dump_root: str = "."):
         dump_name = module_name + "." + dump_format
         with open(Path(dump_root, dump_name), "w") as f:
             f.write(dumps[dump_format])
+
+
+def dump_operation_offloads(mod: tvm.ir.IRModule, initial_relay_astext: list, dump_path: str):
+    """This helper function forms a line-by-line output of the initial Relay lines,
+    indicating which operations are ported to which backend,
+    indicating the composite that includes those operations e.g
+    'device1    <- device2.qnn_conv2d'
+    'device1    <-        %0 = qnn.conv2d(%tfl.quantize, %v_param_1, ...'
+    'device1    <-        %1 = nn.bias_add(%0, %v_param_2, axis=3);'
+    'device1    <-        %2 = qnn.requantize(%1, meta[relay.Constant]...'
+    'device2    <- device2.reshape'
+    'device2    <-        %3 = reshape(%206, newshape=[1, 1001]);'

Review Comment:
   Perhaps its worth showing an example of the generic case here as well?



##########
python/tvm/relay/frontend/common.py:
##########
@@ -997,3 +998,57 @@ def try_resolve_var_to_const(x, graph_params):
         return _op.const(value, dtype)
 
     return x
+
+
+# Span filling to Relay for TFLite taken from
+# commit https://github.com/apache/tvm/pull/9723

Review Comment:
   Its seems like this implementation was reverted in #10072, but #13402 looks to be achieving something similar, do you think it would be possible to rebase this PR ontop of #13402?



##########
python/tvm/driver/tvmc/compiler.py:
##########
@@ -459,3 +493,65 @@ def save_dumps(module_name: str, dumps: Dict[str, str], dump_root: str = "."):
         dump_name = module_name + "." + dump_format
         with open(Path(dump_root, dump_name), "w") as f:
             f.write(dumps[dump_format])
+
+
+def dump_operation_offloads(mod: tvm.ir.IRModule, initial_relay_astext: list, dump_path: str):
+    """This helper function forms a line-by-line output of the initial Relay lines,
+    indicating which operations are ported to which backend,
+    indicating the composite that includes those operations e.g
+    'device1    <- device2.qnn_conv2d'
+    'device1    <-        %0 = qnn.conv2d(%tfl.quantize, %v_param_1, ...'
+    'device1    <-        %1 = nn.bias_add(%0, %v_param_2, axis=3);'
+    'device1    <-        %2 = qnn.requantize(%1, meta[relay.Constant]...'
+    'device2    <- device2.reshape'
+    'device2    <-        %3 = reshape(%206, newshape=[1, 1001]);'
+
+    Parameters
+    ----------
+    mod : tvm.ir.IRModule
+        The IRModule that gets generated from a relay frontend.
+    initial_relay_astext : list
+        List of input model IR strings.
+    dump_path: str
+        Value of the "dump_offloads" compiler atribute.
+        Could be dash ("-") or file path or empty string for
+        printing to console, file or doing nothing respectively.
+    """
+    print_to_console = dump_path == "-"
+    save_to_file = all([dump_path != "-", dump_path != ""])
+
+    if print_to_console or save_to_file:
+
+        operations_distribution = analyze_operations_distribution(mod)
+        output = []
+        prev_op_name = ""
+        for s in initial_relay_astext:
+            result = re.search(r"(output_name: )(.*)(:0:0)(.*)\*/", s)
+            if result:
+                op = result.group(2)
+                hardw_id = ""

Review Comment:
   nit: just to avoid possible confusion, calling this `target_id` might be better as this doesn't strictly need to be different hardware (e.g. `arm_compute_library`)



##########
python/tvm/relay/analysis/operations_distribution.py:
##########
@@ -0,0 +1,86 @@
+# 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.
+"""Utilities that enable analyze Relay and get mappings for unique
+input module layer name to the tuple of compiler and operation name"""
+import tvm
+from tvm import relay
+from tvm.relay.expr_functor import ExprVisitor
+
+
+class AnalyzeOperationsDistribution(ExprVisitor):

Review Comment:
   It would be great if we could add some stand alone tests for this pass



##########
python/tvm/driver/tvmc/compiler.py:
##########
@@ -459,3 +493,65 @@ def save_dumps(module_name: str, dumps: Dict[str, str], dump_root: str = "."):
         dump_name = module_name + "." + dump_format
         with open(Path(dump_root, dump_name), "w") as f:
             f.write(dumps[dump_format])
+
+
+def dump_operation_offloads(mod: tvm.ir.IRModule, initial_relay_astext: list, dump_path: str):
+    """This helper function forms a line-by-line output of the initial Relay lines,
+    indicating which operations are ported to which backend,
+    indicating the composite that includes those operations e.g
+    'device1    <- device2.qnn_conv2d'
+    'device1    <-        %0 = qnn.conv2d(%tfl.quantize, %v_param_1, ...'
+    'device1    <-        %1 = nn.bias_add(%0, %v_param_2, axis=3);'
+    'device1    <-        %2 = qnn.requantize(%1, meta[relay.Constant]...'
+    'device2    <- device2.reshape'
+    'device2    <-        %3 = reshape(%206, newshape=[1, 1001]);'
+
+    Parameters
+    ----------
+    mod : tvm.ir.IRModule
+        The IRModule that gets generated from a relay frontend.
+    initial_relay_astext : list

Review Comment:
   Perhaps I missed it, what's the reason for parsing the initial Relay as a string, rather than traversing a copy of the IRModule?



##########
tests/python/driver/tvmc/test_compiler.py:
##########
@@ -48,6 +51,144 @@ def test_save_dumps(tmpdir_factory):
     assert path.exists("{}/{}".format(tmpdir, "fake_module.relay"))
 
 
+def test_save_dump_offloads_ethosu(tmp_path_factory):
+    def representative_dataset_random_gen():
+        np.random.seed(0)
+        data = np.random.random_sample(input_shape)
+        yield [data.astype(np.float32)]
+
+    tflite = pytest.importorskip("tflite")
+    tensorflow = pytest.importorskip("tensorflow")
+    pytest.importorskip("ethosu.vela")
+
+    from tensorflow.keras import layers
+    from tensorflow.keras.models import Model
+    from tvm.driver.tvmc.model import TVMCModel
+
+    inp = (224, 224, 8)
+    input_shape = (1, *inp)
+
+    inputs = layers.Input(shape=inp)
+    conv_1 = layers.Conv2D(32, (3, 3), padding="same", strides=2)(inputs)
+    dns_1 = layers.Dense(12, activation="linear")(conv_1)
+    outputs = layers.ZeroPadding2D()(dns_1)
+
+    tf_model = Model(inputs, outputs)
+
+    converter = tensorflow.lite.TFLiteConverter.from_keras_model(tf_model)
+    converter.optimizations = [tensorflow.lite.Optimize.DEFAULT]
+    converter.representative_dataset = representative_dataset_random_gen
+    converter.target_spec.supported_ops = [tensorflow.lite.OpsSet.TFLITE_BUILTINS_INT8]
+    converter.inference_input_type = tensorflow.int8
+    converter.inference_output_type = tensorflow.int8
+    tflite_model_buf = converter.convert()

Review Comment:
   nit: Is it possible to make use of?https://github.com/apache/tvm/blob/a4840e7de38c5a2000917f2101f3ec4a374bcd39/tests/python/contrib/test_ethosu/infra.py#L288



-- 
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] lhutton1 commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1039660864


##########
python/tvm/relay/frontend/common.py:
##########
@@ -997,3 +998,57 @@ def try_resolve_var_to_const(x, graph_params):
         return _op.const(value, dtype)
 
     return x
+
+
+# Span filling to Relay for TFLite taken from
+# commit https://github.com/apache/tvm/pull/9723

Review Comment:
   Thanks, yep I think that would be best, it seems like the review is almost finished 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.

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

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


[GitHub] [tvm] chunit-quic commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by "chunit-quic (via GitHub)" <gi...@apache.org>.
chunit-quic commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1090188874


##########
python/tvm/relay/frontend/common.py:
##########
@@ -1067,6 +1067,20 @@ def __init__(self, span):
             self._span = tvm.relay.Span(tvm.relay.SourceName(span.decode("utf-8")), 0, 0, 0, 0)
         else:
             assert False, f"unsupported span type: {type(span)}"
+        self.suffix_str = "_PART_"
+        self.counter = 0
+        self.distance_from_leaf = -1
+
+    def _create_span(self):
+        """Adds suffix_str + counter value to _span.source_name.name,
+        to create a unique source_name for the Relay layer
+        """
+        if self.distance_from_leaf == 0:
+            return tvm.relay.Span(tvm.relay.SourceName(self._span), 0, 0, 0, 0)
+        self.distance_from_leaf -= 1
+        span_str = "{}{}{}".format(self._span.source_name.name, self.suffix_str, str(self.counter))
+        self.counter += 1
+        return tvm.relay.Span(tvm.relay.SourceName(span_str), 0, 0, 0, 0)

Review Comment:
   > This seems sensible to me, cc the original author of span filling @chunit-quic just to check
   
   Hi all,
   
   Thank you for informing us this change, we just check the PR. We have some ideas about it, and we would like to share some discussions about the suffix in the forum with you.
   
   About the way how span is used in the PR, it looks to me like you would like to take the
   source_name of span as an indicator to the original Relay call(expression). 
   We find the hash value might be enough for your use case rather than making a unique source_name for a span. Please allow me to describe some problems might be cuased by adding suffix.
   
   We removed suffix in the new implementation in our [preRFC](https://discuss.tvm.apache.org/t/pre-rfc-tvm-explorer-infrastructure/13457#reference-level-explanation-9). In short, we found adding suffix might mislead in some cases. Here is an example:
   
   Suppose:
   1. An user thinks span meaning where an expression is from
   2. A conv2d layer is converted to 2 relay exprs (conv2d and relu) in a specific frontend.
   
   The conversion of a simple frontend model with only a conv2d layer will result in this:
   
   ```python
   #FRONTEND
   conv2d layer, named as my_conv2d
   #Relay IR
   %0 = conv2d() /* my_conv2d_part_0 */
   %1 = relu() /* my_conv2d_part_1 */
   ```
   In the case above, that user might assume there are "two layers" in the original frontend model, rather than thinking the converted Relay exprs are from the same forntend layer.
   One is conv2d layer with the name "my_conv2d_part_0", and a relu layer which is named as "my_conv2d_part_1".
   
   Yet with help of suffix we do have some advantages. Like allocating the expression which generates the tensor output result of its frontend layer. So far we have [many discussions](https://discuss.tvm.apache.org/t/pre-rfc-tvm-explorer-infrastructure/13457/26?u=chunit) with community but we are still thinking about whether there is a better way to deal with.
   
   Therefore currently we won't recommend you to add suffix to the source_name of span. After checking the codes, perhaps you could consider using the hash value, `tvm.ir.structural_hash()` of an expr as the key of dictionary, like:
   
   ```python
   #python/tvm/relay/analysis/operations_distribution.py
   class AnalyzeOperationsDistribution(ExprVisitor):
       #...
       def visit_call(self, call: relay.Call):
           if isinstance(call.op, tvm.ir.Op):
               self.unique_op_ids[tvm.ir.structural_hash(call)] = [self.compiler_name, self.func_name, self.func_id]
   #python/tvm/driver/tvmc/compiler.py
   def dump_operation_offloads(...):
       def annotate_f(x):
           ret = ""
           if isinstance(x, relay.Call):
               compiler_name, op_name, func_id = operations_distribution[tvm.ir.structural_hash(x)]
               ret = f", func_id: {func_id}, compiler_name: {compiler_name}, op_name: {op_name}"
           return ret
   ```
   
   Because no rewriting/mutation is invoked in `dump_operation_offloads()`, hash value won't be changed. Maybe hash value is an acceptable alternative?
   
   Thank you for reading! Feel free if you have any question. We will try to get back to you soon. :)
   
   



-- 
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] arina-grovety commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by "arina-grovety (via GitHub)" <gi...@apache.org>.
arina-grovety commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1147410067


##########
python/tvm/relay/analysis/operations_distribution.py:
##########
@@ -0,0 +1,86 @@
+# 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.
+"""Utilities that enable analyze Relay and get mappings for unique
+input module layer name to the tuple of compiler and operation name"""
+import tvm
+from tvm import relay
+from tvm.relay.expr_functor import ExprVisitor
+
+
+class AnalyzeOperationsDistribution(ExprVisitor):

Review Comment:
   Hello @lhutton1, thanks for the reminder, added a test.
   



-- 
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] sergio-grovety commented on pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

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

   @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] lhutton1 commented on pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

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

   Thanks @sergio-grovety @arina-grovety @chunit-quic @ashutosh-arm! This will be very helpful for users wanting to see how their models were offloaded, thanks for persisting with all the changes!


-- 
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] arina-grovety commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by "arina-grovety (via GitHub)" <gi...@apache.org>.
arina-grovety commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1084048146


##########
tests/python/driver/tvmc/test_compiler.py:
##########
@@ -48,6 +51,144 @@ def test_save_dumps(tmpdir_factory):
     assert path.exists("{}/{}".format(tmpdir, "fake_module.relay"))
 
 
+def test_save_dump_offloads_ethosu(tmp_path_factory):
+    def representative_dataset_random_gen():
+        np.random.seed(0)
+        data = np.random.random_sample(input_shape)
+        yield [data.astype(np.float32)]
+
+    tflite = pytest.importorskip("tflite")
+    tensorflow = pytest.importorskip("tensorflow")
+    pytest.importorskip("ethosu.vela")
+
+    from tensorflow.keras import layers
+    from tensorflow.keras.models import Model
+    from tvm.driver.tvmc.model import TVMCModel
+
+    inp = (224, 224, 8)
+    input_shape = (1, *inp)
+
+    inputs = layers.Input(shape=inp)
+    conv_1 = layers.Conv2D(32, (3, 3), padding="same", strides=2)(inputs)
+    dns_1 = layers.Dense(12, activation="linear")(conv_1)
+    outputs = layers.ZeroPadding2D()(dns_1)
+
+    tf_model = Model(inputs, outputs)
+
+    converter = tensorflow.lite.TFLiteConverter.from_keras_model(tf_model)
+    converter.optimizations = [tensorflow.lite.Optimize.DEFAULT]
+    converter.representative_dataset = representative_dataset_random_gen
+    converter.target_spec.supported_ops = [tensorflow.lite.OpsSet.TFLITE_BUILTINS_INT8]
+    converter.inference_input_type = tensorflow.int8
+    converter.inference_output_type = tensorflow.int8
+    tflite_model_buf = converter.convert()

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] arina-grovety commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by "arina-grovety (via GitHub)" <gi...@apache.org>.
arina-grovety commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1089071751


##########
python/tvm/driver/tvmc/compiler.py:
##########
@@ -459,3 +493,65 @@ def save_dumps(module_name: str, dumps: Dict[str, str], dump_root: str = "."):
         dump_name = module_name + "." + dump_format
         with open(Path(dump_root, dump_name), "w") as f:
             f.write(dumps[dump_format])
+
+
+def dump_operation_offloads(mod: tvm.ir.IRModule, initial_relay_astext: list, dump_path: str):
+    """This helper function forms a line-by-line output of the initial Relay lines,
+    indicating which operations are ported to which backend,
+    indicating the composite that includes those operations e.g
+    'device1    <- device2.qnn_conv2d'
+    'device1    <-        %0 = qnn.conv2d(%tfl.quantize, %v_param_1, ...'
+    'device1    <-        %1 = nn.bias_add(%0, %v_param_2, axis=3);'
+    'device1    <-        %2 = qnn.requantize(%1, meta[relay.Constant]...'
+    'device2    <- device2.reshape'
+    'device2    <-        %3 = reshape(%206, newshape=[1, 1001]);'
+
+    Parameters
+    ----------
+    mod : tvm.ir.IRModule
+        The IRModule that gets generated from a relay frontend.
+    initial_relay_astext : list

Review Comment:
   Hello @ashutosh-arm,
   sorry, there is a non-fixed comment,
   
   now we pass the initial Relay as Relay IR itself, and then use "annotate" parameter of  the [astext()](https://github.com/apache/tvm/pull/13212/files#diff-5c75738c5cc53888ceb0c8b4833a013631295fb70b2a3a1178f8523271052671R536) function 
   to add the desired annotations to the generated text, and then parsing our annotations from the formed text.
   
   I will fix the comment string in the update to the 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.

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

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


[GitHub] [tvm] lhutton1 commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

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


##########
python/tvm/relay/frontend/common.py:
##########
@@ -1067,6 +1067,20 @@ def __init__(self, span):
             self._span = tvm.relay.Span(tvm.relay.SourceName(span.decode("utf-8")), 0, 0, 0, 0)
         else:
             assert False, f"unsupported span type: {type(span)}"
+        self.suffix_str = "_PART_"
+        self.counter = 0
+        self.distance_from_leaf = -1
+
+    def _create_span(self):
+        """Adds suffix_str + counter value to _span.source_name.name,
+        to create a unique source_name for the Relay layer
+        """
+        if self.distance_from_leaf == 0:
+            return tvm.relay.Span(tvm.relay.SourceName(self._span), 0, 0, 0, 0)
+        self.distance_from_leaf -= 1
+        span_str = "{}{}{}".format(self._span.source_name.name, self.suffix_str, str(self.counter))
+        self.counter += 1
+        return tvm.relay.Span(tvm.relay.SourceName(span_str), 0, 0, 0, 0)

Review Comment:
   Apologies for only just getting back to this, yep I think the hash on the call won't work due to difference between `initial_mod` and `mod` caused by mutation during the partitioning process :/ I had a read through the span filling preRFC and it does indeed sound like a complex issue, but I think the reasoning for removing the suffix is valid (see [here](https://discuss.tvm.apache.org/t/pre-rfc-tvm-explorer-infrastructure/13457/29#how-many-benefits-can-we-get-from-suffix-2)), so I'm a bit hesitant to re-introduce it, but would be good to hear @chunit-quic's opinion.
   
   A naive and not well thought through suggestion, but I thought I'd throw it out there - since spans don't seem to be mutated during the lowering, is it possible to make span uniquely hashable, perhaps by introducing some kind of id to the SpanNode and therefore keeping the name intact? 
   
   



-- 
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] arina-grovety commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by "arina-grovety (via GitHub)" <gi...@apache.org>.
arina-grovety commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1114164082


##########
python/tvm/relay/frontend/common.py:
##########
@@ -1067,6 +1067,20 @@ def __init__(self, span):
             self._span = tvm.relay.Span(tvm.relay.SourceName(span.decode("utf-8")), 0, 0, 0, 0)
         else:
             assert False, f"unsupported span type: {type(span)}"
+        self.suffix_str = "_PART_"
+        self.counter = 0
+        self.distance_from_leaf = -1
+
+    def _create_span(self):
+        """Adds suffix_str + counter value to _span.source_name.name,
+        to create a unique source_name for the Relay layer
+        """
+        if self.distance_from_leaf == 0:
+            return tvm.relay.Span(tvm.relay.SourceName(self._span), 0, 0, 0, 0)
+        self.distance_from_leaf -= 1
+        span_str = "{}{}{}".format(self._span.source_name.name, self.suffix_str, str(self.counter))
+        self.counter += 1
+        return tvm.relay.Span(tvm.relay.SourceName(span_str), 0, 0, 0, 0)

Review Comment:
   Hello @chunit-quic,
   thank you for the detailed explanations!
   
   I'm sorry, but I'm a bit confused. Let me explain my question:
   
   First, do I understand correctly that the workflow in my case will be as follows:
   
   frontend model -> Relay IR (IR1)
   if --dump-offloads option is on
        tag IR1 with suffixes using the _SuffixTagger pass
   inside the tvmc.compiler.compile_model:
           ...
           IR1(optionally tagged) transfoms by partitions for selected backends (IR2)
           if --dump-offloads option is on
               dump initial_relay offloaded result
           ... 
           
   And here is my question:
   If I understand correctly, the suffix, although optional, sometimes will still be present in the Relay.
   Doesn't this mean that, although optional, we will again get the situation we wanted to avoid by removing the suffixes?



-- 
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] arina-grovety commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by "arina-grovety (via GitHub)" <gi...@apache.org>.
arina-grovety commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1114164082


##########
python/tvm/relay/frontend/common.py:
##########
@@ -1067,6 +1067,20 @@ def __init__(self, span):
             self._span = tvm.relay.Span(tvm.relay.SourceName(span.decode("utf-8")), 0, 0, 0, 0)
         else:
             assert False, f"unsupported span type: {type(span)}"
+        self.suffix_str = "_PART_"
+        self.counter = 0
+        self.distance_from_leaf = -1
+
+    def _create_span(self):
+        """Adds suffix_str + counter value to _span.source_name.name,
+        to create a unique source_name for the Relay layer
+        """
+        if self.distance_from_leaf == 0:
+            return tvm.relay.Span(tvm.relay.SourceName(self._span), 0, 0, 0, 0)
+        self.distance_from_leaf -= 1
+        span_str = "{}{}{}".format(self._span.source_name.name, self.suffix_str, str(self.counter))
+        self.counter += 1
+        return tvm.relay.Span(tvm.relay.SourceName(span_str), 0, 0, 0, 0)

Review Comment:
   Hello @chunit-quic,
   thank you for the detailed explanations!
   
   I'm sorry, but I'm a bit confused. Let me explain my question:
   
   First, do I understand correctly that the workflow in my case will be as follows:
   
   frontend model -> Relay IR (IR1)
   if --dump-offloads option is on
   &nbsp;&nbsp;  tag IR1 with suffixes using the _SuffixTagger pass
   inside the tvmc.compiler.compile_model:
   &nbsp;&nbsp;  ...
   &nbsp;&nbsp;  IR1(optionally tagged) transfoms by partitions for selected backends (IR2)
   &nbsp;&nbsp;  if --dump-offloads option is on
   &nbsp;&nbsp;&nbsp;&nbsp;  dump initial_relay offloaded result
   &nbsp;&nbsp;  ... 
           
   And here is my question:
   If I understand correctly, the suffix, although optional, sometimes will still be present in the Relay.
   Doesn't this mean that, although optional, we will again get the situation we wanted to avoid by removing the suffixes?



-- 
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] lhutton1 merged pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

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


-- 
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] arina-grovety commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
arina-grovety commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1038029576


##########
python/tvm/driver/tvmc/compiler.py:
##########
@@ -459,3 +493,65 @@ def save_dumps(module_name: str, dumps: Dict[str, str], dump_root: str = "."):
         dump_name = module_name + "." + dump_format
         with open(Path(dump_root, dump_name), "w") as f:
             f.write(dumps[dump_format])
+
+
+def dump_operation_offloads(mod: tvm.ir.IRModule, initial_relay_astext: list, dump_path: str):
+    """This helper function forms a line-by-line output of the initial Relay lines,
+    indicating which operations are ported to which backend,
+    indicating the composite that includes those operations e.g
+    'device1    <- device2.qnn_conv2d'
+    'device1    <-        %0 = qnn.conv2d(%tfl.quantize, %v_param_1, ...'
+    'device1    <-        %1 = nn.bias_add(%0, %v_param_2, axis=3);'
+    'device1    <-        %2 = qnn.requantize(%1, meta[relay.Constant]...'
+    'device2    <- device2.reshape'
+    'device2    <-        %3 = reshape(%206, newshape=[1, 1001]);'
+
+    Parameters
+    ----------
+    mod : tvm.ir.IRModule
+        The IRModule that gets generated from a relay frontend.
+    initial_relay_astext : list

Review Comment:
   Hi @lhutton1,
   this was done to avoid copying entities that we consider unnecessary, since only the text representation of the Relay is used in this function



-- 
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] lhutton1 commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

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


##########
python/tvm/driver/tvmc/compiler.py:
##########
@@ -459,3 +493,65 @@ def save_dumps(module_name: str, dumps: Dict[str, str], dump_root: str = "."):
         dump_name = module_name + "." + dump_format
         with open(Path(dump_root, dump_name), "w") as f:
             f.write(dumps[dump_format])
+
+
+def dump_operation_offloads(mod: tvm.ir.IRModule, initial_relay_astext: list, dump_path: str):
+    """This helper function forms a line-by-line output of the initial Relay lines,
+    indicating which operations are ported to which backend,
+    indicating the composite that includes those operations e.g
+    'device1    <- device2.qnn_conv2d'
+    'device1    <-        %0 = qnn.conv2d(%tfl.quantize, %v_param_1, ...'
+    'device1    <-        %1 = nn.bias_add(%0, %v_param_2, axis=3);'
+    'device1    <-        %2 = qnn.requantize(%1, meta[relay.Constant]...'
+    'device2    <- device2.reshape'
+    'device2    <-        %3 = reshape(%206, newshape=[1, 1001]);'
+
+    Parameters
+    ----------
+    mod : tvm.ir.IRModule
+        The IRModule that gets generated from a relay frontend.
+    initial_relay_astext : list

Review Comment:
   Apologies if there might have been some confusion here, this question was more around the need to search over the relay IR as text for the compiler name, op name, func id, etc. The information could be extracted using a visitor pass (`ExprVisitor`) that traverses the IR, making it more resilient to changes in the text format of the IR. Since this method is working and to move this forwards, we can pull this out into a separate follow-up



-- 
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] chunit-quic commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by "chunit-quic (via GitHub)" <gi...@apache.org>.
chunit-quic commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1145618964


##########
tests/python/contrib/test_ethosu/test_pass_operations_distribution.py:
##########
@@ -0,0 +1,87 @@
+# 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.
+from tvm.relay.analysis.operations_distribution import analyze_operations_distribution
+from tvm.relay.transform.suffixes import tag_suffixes
+from . import infra
+import pytest
+import numpy as np
+
+
+def test_operations_distribution():
+
+    tflite = pytest.importorskip("tflite")
+    tensorflow = pytest.importorskip("tensorflow")
+    pytest.importorskip("ethosu.vela")
+
+    import tensorflow as tf
+
+    inp = (224, 224, 9)
+    input_shape = (1, *inp)
+    kernel_shape = (3, 3)
+    padding = (1, 1, 1, 1)
+    padding_out = (1, 33, 33, 1)
+
+    @tf.function
+    def simple_net(x):
+        weight_shape = [kernel_shape[0], kernel_shape[1], input_shape[3], 3]
+        weights = tf.constant(np.random.uniform(size=weight_shape), dtype=tf.float32)
+        op = tf.nn.conv2d(
+            x,
+            filters=weights,
+            strides=1,
+            padding="SAME",
+            data_format="NHWC",
+            dilations=1,
+        )
+        op = tf.pad(
+            op,
+            [[0, 0], [padding[0], padding_out[2]], [padding_out[1], padding[3]], [0, 0]],
+            "CONSTANT",
+        )
+        op = tf.pad(
+            op,
+            [[0, 0], [padding[0], padding[2]], [padding[1], padding[3]], [0, 0]],
+            "CONSTANT",
+        )
+        return tf.pad(
+            op,
+            [[0, 0], [padding_out[0], padding[2]], [padding[1], padding_out[3]], [0, 0]],
+            "CONSTANT",
+        )
+
+    from tests.python.contrib.test_ethosu.infra import get_tflite_graph

Review Comment:
   Perhaps it might be good to add this try catch for tflite import in infra.py:
   ```python
   # Get TFLite model from buffer
   try:
       import tflite
   
       tflite_model = tflite.Model.GetRootAsModel([tflite_model_buf], 0)
   except AttributeError:
       import tflite.Model
       tflite_model = tflite.Model.Model.GetRootAsModel([tflite_model_buf], 0)
   ````
   https://tvm.apache.org/docs/how_to/compile_models/from_tflite.html#load-pretrained-tflite-model



-- 
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] arina-grovety commented on pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

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

   > @tvm-bot rerun
   
   Hello @lhutton1, thank 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.

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

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


[GitHub] [tvm] arina-grovety commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by "arina-grovety (via GitHub)" <gi...@apache.org>.
arina-grovety commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1102871792


##########
python/tvm/relay/frontend/common.py:
##########
@@ -1067,6 +1067,20 @@ def __init__(self, span):
             self._span = tvm.relay.Span(tvm.relay.SourceName(span.decode("utf-8")), 0, 0, 0, 0)
         else:
             assert False, f"unsupported span type: {type(span)}"
+        self.suffix_str = "_PART_"
+        self.counter = 0
+        self.distance_from_leaf = -1
+
+    def _create_span(self):
+        """Adds suffix_str + counter value to _span.source_name.name,
+        to create a unique source_name for the Relay layer
+        """
+        if self.distance_from_leaf == 0:
+            return tvm.relay.Span(tvm.relay.SourceName(self._span), 0, 0, 0, 0)
+        self.distance_from_leaf -= 1
+        span_str = "{}{}{}".format(self._span.source_name.name, self.suffix_str, str(self.counter))
+        self.counter += 1
+        return tvm.relay.Span(tvm.relay.SourceName(span_str), 0, 0, 0, 0)

Review Comment:
   Hello @chunit-quic.
   
   Unfortunately, in my case the value of tvm.ir.structural_hash(call) given at AnalyzeOperationsDistribution:visit_call()
   differ from the value of tvm.ir.structural_hash(x) at dump_operation_offloads:annotate_f().
   
   Our task was to print in a human-readable form, which operators from the original graph are offloaded to Ethos-u and which aren't. 
   To do this, we collected the information from the Relay after Partition and annotated the original Relay (it is implemented in this PR). And the key point was that we used a suffix to obtain a unique identifier that associates the string of the original Relay with the operator from the resulting Relay. 
   
   But the suffixes will be removed, and the hash does not match.
   It looks like we don't have the right mechanism yet to associate the original Relay call with the resulting Relay.
   
   We probably need to wait a decision for this [preRFC](https://discuss.tvm.apache.org/t/pre-rfc-tvm-explorer-infrastructure/13457#reference-level-explanation-9). 



-- 
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] arina-grovety commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by "arina-grovety (via GitHub)" <gi...@apache.org>.
arina-grovety commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1084047924


##########
python/tvm/relay/analysis/operations_distribution.py:
##########
@@ -0,0 +1,86 @@
+# 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.
+"""Utilities that enable analyze Relay and get mappings for unique
+input module layer name to the tuple of compiler and operation name"""
+import tvm
+from tvm import relay
+from tvm.relay.expr_functor import ExprVisitor
+
+
+class AnalyzeOperationsDistribution(ExprVisitor):

Review Comment:
   Done. Please review again. 
   @lhutton1 



-- 
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] chunit-quic commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by "chunit-quic (via GitHub)" <gi...@apache.org>.
chunit-quic commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1116451045


##########
python/tvm/relay/frontend/common.py:
##########
@@ -1067,6 +1067,20 @@ def __init__(self, span):
             self._span = tvm.relay.Span(tvm.relay.SourceName(span.decode("utf-8")), 0, 0, 0, 0)
         else:
             assert False, f"unsupported span type: {type(span)}"
+        self.suffix_str = "_PART_"
+        self.counter = 0
+        self.distance_from_leaf = -1
+
+    def _create_span(self):
+        """Adds suffix_str + counter value to _span.source_name.name,
+        to create a unique source_name for the Relay layer
+        """
+        if self.distance_from_leaf == 0:
+            return tvm.relay.Span(tvm.relay.SourceName(self._span), 0, 0, 0, 0)
+        self.distance_from_leaf -= 1
+        span_str = "{}{}{}".format(self._span.source_name.name, self.suffix_str, str(self.counter))
+        self.counter += 1
+        return tvm.relay.Span(tvm.relay.SourceName(span_str), 0, 0, 0, 0)

Review Comment:
   Hi @arina-grovety ,
   
   > First, do I understand correctly that the workflow in my case will be as follows:
   
   Yes, invoke the post-processing pass to tag suffix to IR1 should result in the same effect as your modification currently. Which is that the relation between each span and expression is one-to-one mapping.
   
   One more thing is, sorry that perhaps I forgot to deal a condition in the previous pseudo code. We might need a dict to record whether the span of an expression is rewritten. If yes, return the written expression, else keep visiting the expression. Otherwise, we might rewrite the span of an expression which has many consumers mutiple times.
   
   > Doesn't this mean that, although optional, we will again get the situation we wanted to avoid by removing the suffixes?
   
   Unfortunately yes. The post-processing way is a workaround specifically for this case. We will still face the problems introduced by suffix, even though it is optional. Currently we use the span, or to be more precisely, the source_name attribute of span as the one and only one indicator to the source of an expression. Somehow it might be OK for the mapping between `frontend source(TF, ONNX...) <-> Relay IR (with passes)`. 
   However, we observe that in some use cases, what we ask for span is much more than what it can provide to us currently. Like:
   
   1. Be an unique indicator for us to obtain which expression a span belongs to.
   2. Indicate which expression produces the output tensor of its frontend source layer in one-to-many conversion.
   3. Construct precise mapping between a pass
   
   Including the issue 1 we have in here, we also have some discussions in the preRFC try to handle the issues above, for example.
   [line column number for 3.](https://discuss.tvm.apache.org/t/pre-rfc-tvm-explorer-infrastructure/13457#unresolved-questions-19)
   [A var tracing map for 2](https://discuss.tvm.apache.org/t/pre-rfc-tvm-explorer-infrastructure/13457/17?u=chunit)
   [add attribute or custom interface for 2.](https://discuss.tvm.apache.org/t/pre-rfc-tvm-explorer-infrastructure/13457/32#1-extend-the-object-span-add-an-attribute-1)
   
   Because both expressions and spans might be changed, merged or even discarded during the pass transformation, it is hardly to reach all expectations with the capacity of current span instance. Until now we have gathered some idea, yet it is still far from the implementation. Therefore currently a post-processing workaround might be an short-term alternative before a proper mechanism is 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] arina-grovety commented on pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
arina-grovety commented on PR #13212:
URL: https://github.com/apache/tvm/pull/13212#issuecomment-1329823628

   > 
   > Thanks for the explanation @arina-grovety, yes I was thinking other backends like CMSIS-NN could make use of the same approach since the `AnalyzeOperationsDistribution` already seems quite generic. Where possible we can add the composite function names and if they are not found we can fallback to just printing the Relay - exactly as you described. If another backend has a different method of offloading operations this could simply be added to the pass in the future as and when needed.
   > 
   
   Hello @lhutton1, we have pushed update to the PR, option "--target-ethos-u-dump_npu_functions_coverage" has been replaced by more generic "--dump-offloads" with the same meaning.
   


-- 
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] arina-grovety commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by GitBox <gi...@apache.org>.
arina-grovety commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1037976787


##########
python/tvm/relay/frontend/common.py:
##########
@@ -997,3 +998,57 @@ def try_resolve_var_to_const(x, graph_params):
         return _op.const(value, dtype)
 
     return x
+
+
+# Span filling to Relay for TFLite taken from
+# commit https://github.com/apache/tvm/pull/9723

Review Comment:
   Hi @lhutton1, thank you for reviewing!
   We've corrected a few points based on your previous comments.
   
   
   
   > Its seems like this implementation was reverted in #10072, but #13402 looks to be achieving something similar, do you think it would be possible to rebase this PR ontop of #13402?
   
   Checked the changes in #13402, some are the same as our PR and some are different, so I think it may be worth waiting for #13402 to be accepted?



-- 
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] chunit-quic commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by "chunit-quic (via GitHub)" <gi...@apache.org>.
chunit-quic commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1109437322


##########
python/tvm/relay/frontend/common.py:
##########
@@ -1067,6 +1067,20 @@ def __init__(self, span):
             self._span = tvm.relay.Span(tvm.relay.SourceName(span.decode("utf-8")), 0, 0, 0, 0)
         else:
             assert False, f"unsupported span type: {type(span)}"
+        self.suffix_str = "_PART_"
+        self.counter = 0
+        self.distance_from_leaf = -1
+
+    def _create_span(self):
+        """Adds suffix_str + counter value to _span.source_name.name,
+        to create a unique source_name for the Relay layer
+        """
+        if self.distance_from_leaf == 0:
+            return tvm.relay.Span(tvm.relay.SourceName(self._span), 0, 0, 0, 0)
+        self.distance_from_leaf -= 1
+        span_str = "{}{}{}".format(self._span.source_name.name, self.suffix_str, str(self.counter))
+        self.counter += 1
+        return tvm.relay.Span(tvm.relay.SourceName(span_str), 0, 0, 0, 0)

Review Comment:
   Hi @arina-grovety and @lhutton1 
   
   Thank you for informing us at once :D. Sorry that I didn't notice the rewriting part in the invoked pass. Before going to details, our suggestion will be "write a post-processing to tag span suffix" specifically for this kind of scenario. So that we could have a workaround and still keep suffix being discarded as default.
   Here are the reasons in short below:
   1. We still cannot find methods to handle the impacts after introducing the suffix as default.
   2. Span somehow will be changed according to the functionality of pass.
   
   Now I would like to go through the process about why we would suggest writing the post-processing pass. I will try to describe the goals of this PR, and what problems might be encountered. Please kindly correct me if I still miss something. :)
   
   # Goal
   Obtain the mapping between the offloaded result and the frontend operators. The flow should be:
   ```
   frontend model ->
   Relay IR (IR1) ->
   Relay IR transfomred by partition_for_ethosu() (IR2)->
   tvmc.complie_model ->
   Relay IR transfomred by convert_graph_layout and partition_functions (IR3)->
   dump initial_relay offloaded result
   ```
   Based on the codes PR([compiler.py:347](https://github.com/apache/tvm/blob/325b2ef24496eadc14836cb2c8a168a06dff37e8/python/tvm/driver/tvmc/compiler.py#L347)), we would like to get the mapping via IR3 and IR2.
   
   # Requirement
   We need a unique indicator to maintain the relation offloaded result. If the `source_name` of `span` is unique, it might be a choice. Therefore here comes two proposals:
   1. Add suffix to span as default.
   2. Add unique id to span.
   
   # Challenges we will encounter
   In this section we would try to describe what challenges we encountered. Since the expressions are modified by partition_functions, we cannot use the hash key of expressions. Now let's check the proposals in the previous section.
   
   1. Add suffix to span as default.  
     The drawback of setting suffix as default is still significant. Like the discussions in the [preRFC](https://discuss.tvm.apache.org/t/pre-rfc-tvm-explorer-infrastructure/13457/29#how-many-benefits-can-we-get-from-suffix-2).
     We still don't encourage to add it back as default.
   2. Add unique id to span  
     An obvious obstacle we will face is, it becomes tough to compare the equality between span once we   add an unique id. Another problem is, with the change of expressions, span might be changed also   according to the functionality of pass.
   Take the concept of foldconstant pass for example, consider the following transformation:
     ```python
     IR1:
       fn(%input) {
         %0 = add(meta[relay.Constant] /* span=const_1*/, meta[relay.Constant] /* span=const_2*/]) /* span=add*/
         %1 = subtract(%input, %0) /* span=sub */
       }
     # After foldconstant pass it should look like
     IR2:
       fn(%input) {
         %0 = subtract(%input, meta[relay.Constant] /* span=[const_1, const_2, add]*/) /* span=sub */
       }
     ```
     The span of the folded constant expressions is from two Constant and an add Call. Therefore, even   if we have the unique id of const_1, const_2 and add span, we still cannot have a better experience to get the relation.
   
     Note that if you are interested in how to construct a many-to-one span, it is [one of the ongoing features](https://discuss.tvm.apache.org/t/pre-rfc-tvm-explorer-infrastructure/13457#pass-source-information-builder-6) in the preRFC of TVM Explorer infra. We will try to submit that PR recently.
   
   # Conclusion
   From my point of view, it is still challenging for us to add back the suffix, and unique ID
   for span might not be easy to deal with either. However, we do observe that making span as an unique indicator is quite useful in some cases. Therefore we will suggest writing a distinct pass to tag suffix to the span. With the help of the suffix-tagging pass, we can have a flexibility to work with and still keep the suffix problem away from the build flow by default.
   
   Here is a sudo persudo code for your reference. 
   ```python
   from collections import defaultdict
   class _SuffixTagger(ExprMutator):
       """_SuffixTagger"""
       def __init__(self):
           ExprMutator.__init__(self)
           # key: span or source name, value: counter, indexed from 0
           self.lookup = defaultdict(int)
           self.suffix = "_PART_"
       def _tag_suffix(self, span):
           # To avoid error once we introduce the SequentialSpan in the future
           #https://discuss.tvm.apache.org/t/pre-rfc-tvm-explorer-infrastructure/13457#pass-source-information-builder-6
           # Don't need this if currently
           if isinstance(span, relay.Span):
               ori_name = span.source_name.name
               new_name = ori_name + self.suffix + self.lookup[ori_name]
               lookup[ori_name] += 1
               return tvm.relay.Span(
                   tvm.relay.SourceName(new_name), span.line, span.end_line, span.column, span.end_column
               )
           return span
       def visit(self, expr):
           if hasattr(expr, "span"):
               return super().visit(expr)
       def visit_call(self, call):
           new_args = [self.visit(arg) for arg in call.args]
           new_op = self.visit(call.op)
           return _expr.CallWithFields(
               call, new_op, new_args, call.attrs, call.type_args, None, self._tag_suffix()
           )
   ```
   
   Thank you for reading such long post. If there is any question please don't hesitate to let me know. I will get back to you soon. :D



-- 
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] lhutton1 commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

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


##########
python/tvm/relay/analysis/operations_distribution.py:
##########
@@ -0,0 +1,86 @@
+# 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.
+"""Utilities that enable analyze Relay and get mappings for unique
+input module layer name to the tuple of compiler and operation name"""
+import tvm
+from tvm import relay
+from tvm.relay.expr_functor import ExprVisitor
+
+
+class AnalyzeOperationsDistribution(ExprVisitor):

Review Comment:
   nit: did a generic only test get added?



##########
tests/python/driver/tvmc/test_compiler.py:
##########
@@ -49,6 +51,355 @@ def test_save_dumps(tmpdir_factory):
     assert path.exists("{}/{}".format(tmpdir, "fake_module.relay"))
 
 
+def test_save_dump_offloads_ethosu(tmp_path_factory):
+
+    tflite = pytest.importorskip("tflite")
+    tensorflow = pytest.importorskip("tensorflow")
+    pytest.importorskip("ethosu.vela")
+
+    import tensorflow as tf
+    import tflite.Model
+    from tvm.driver.tvmc.model import TVMCModel
+
+    inp = (224, 224, 9)
+    input_shape = (1, *inp)
+    kernel_shape = (3, 3)
+    padding = (1, 1, 1, 1)
+    padding_out = (1, 33, 33, 1)
+
+    @tf.function
+    def simple_net(x):
+        weight_shape = [kernel_shape[0], kernel_shape[1], input_shape[3], 3]
+        weights = tf.constant(np.random.uniform(size=weight_shape), dtype=tf.float32)
+        weight_shape[2] = 3
+        weights1 = tf.constant(np.random.uniform(size=weight_shape), dtype=tf.float32)
+        weights2 = tf.constant(np.random.uniform(size=weight_shape), dtype=tf.float32)
+        op = tf.nn.conv2d(
+            x,
+            filters=weights,
+            strides=1,
+            padding="SAME",
+            data_format="NHWC",
+            dilations=1,
+        )
+        op1 = tf.nn.conv2d(
+            op,
+            filters=weights1,
+            strides=1,
+            padding="SAME",
+            data_format="NHWC",
+            dilations=1,
+        )
+        op2 = tf.nn.conv2d(
+            op,
+            filters=weights2,
+            strides=1,
+            padding="SAME",
+            data_format="NHWC",
+            dilations=1,
+        )
+        op = tf.math.add(op1, op2)
+        op = tf.pad(
+            op,
+            [[0, 0], [padding[0], padding_out[1]], [padding_out[2], padding[3]], [0, 0]],
+            "CONSTANT",
+        )
+        return op
+
+    from tests.python.contrib.test_ethosu.infra import get_tflite_graph
+
+    _, tflite_graph = get_tflite_graph(simple_net, [input_shape])
+    tflite_model = tflite.Model.Model.GetRootAsModel(tflite_graph, 0)
+    mod, params = relay.frontend.from_tflite(tflite_model)
+
+    tvmc_model = TVMCModel(mod, params)
+
+    output_dir = tmp_path_factory.mktemp("tmp")
+    output_file_name = os.path.join(str(output_dir), "list.txt")
+
+    tvmc.compiler.compile_model(
+        tvmc_model,
+        target="ethos-u,cmsis-nn,c",
+        runtime=Runtime("crt"),
+        tuning_records="",
+        package_path="module.tar",
+        executor=Executor("aot", {"unpacked-api": 1, "interface-api": "c", "link-params": True}),
+        cross="",
+        cross_options="",
+        output_format="mlf",
+        dump_offloads=output_file_name,
+        disabled_pass=[""],
+        pass_context_configs=[
+            "tir.disable_vectorize=1",
+            "tir.usmp.enable=1",
+            "tir.usmp.algorithm=hill_climb",
+            "tir.disable_storage_rewrite=1",
+            "relay.frontend.fill_span=1",
+        ],
+        additional_target_options={
+            "c": {"mcpu": "cortex-m55"},
+            "cmsis-nn": {"mcpu": "cortex-m55"},
+            "ethos-u": {
+                "accelerator_config": "ethos-u55-256",
+            },
+        },
+    )
+
+    expected = [
+        r"Total number of operators and distribution by targets",
+        r"Total: 11",
+        r"ethos-u: 10",
+        r"generic: 1",

Review Comment:
   I didn't notice this before, looks good :)



-- 
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] arina-grovety commented on pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

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

   Hello @lhutton1, thank 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.

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

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


[GitHub] [tvm] lhutton1 commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

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


##########
python/tvm/driver/tvmc/compiler.py:
##########
@@ -459,3 +489,79 @@ def save_dumps(module_name: str, dumps: Dict[str, str], dump_root: str = "."):
         dump_name = module_name + "." + dump_format
         with open(Path(dump_root, dump_name), "w") as f:
             f.write(dumps[dump_format])
+
+
+def dump_operation_offloads(mod: tvm.ir.IRModule, initial_mod: tvm.ir.IRModule, dump_path: str):
+    """This helper function forms a line-by-line output of the initial Relay lines,
+    indicating which operations are ported to which target,
+    and indicating the composite that includes those operations;
+    the 'generic' target refers to operations uploaded to the host, e.g
+    'target1        <-     target2.qnn_conv2d'

Review Comment:
   nit: `target2.qnn_conv2d` -> `target1.qnn_conv2d`



##########
python/tvm/relay/analysis/operations_distribution.py:
##########
@@ -0,0 +1,86 @@
+# 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.
+"""Utilities that enable analyze Relay and get mappings for unique
+input module layer name to the tuple of compiler and operation name"""
+import tvm
+from tvm import relay
+from tvm.relay.expr_functor import ExprVisitor
+
+
+class AnalyzeOperationsDistribution(ExprVisitor):

Review Comment:
   Thanks, could we also add a test for the generic only fallback case?



##########
python/tvm/relay/frontend/common.py:
##########
@@ -1067,6 +1067,20 @@ def __init__(self, span):
             self._span = tvm.relay.Span(tvm.relay.SourceName(span.decode("utf-8")), 0, 0, 0, 0)
         else:
             assert False, f"unsupported span type: {type(span)}"
+        self.suffix_str = "_PART_"
+        self.counter = 0
+        self.distance_from_leaf = -1
+
+    def _create_span(self):
+        """Adds suffix_str + counter value to _span.source_name.name,
+        to create a unique source_name for the Relay layer
+        """
+        if self.distance_from_leaf == 0:
+            return tvm.relay.Span(tvm.relay.SourceName(self._span), 0, 0, 0, 0)
+        self.distance_from_leaf -= 1
+        span_str = "{}{}{}".format(self._span.source_name.name, self.suffix_str, str(self.counter))
+        self.counter += 1
+        return tvm.relay.Span(tvm.relay.SourceName(span_str), 0, 0, 0, 0)

Review Comment:
   This seems sensible to me, cc the original author of span filling @chunit-quic just to check
   
   Is `self.counter` required, or can the same info already be fetched from `self.distance_from_leaf` e.g. `.format(..., str(-self.distance_from_leaf))`?



##########
tests/python/contrib/test_ethosu/test_pass_operations_distribution.py:
##########
@@ -0,0 +1,84 @@
+# 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.
+from tvm.relay.analysis.operations_distribution import analyze_operations_distribution
+from . import infra
+import pytest
+import numpy as np
+
+
+def test_operations_distribution():
+
+    tflite = pytest.importorskip("tflite")
+    tensorflow = pytest.importorskip("tensorflow")
+    pytest.importorskip("ethosu.vela")
+
+    import tensorflow as tf
+
+    inp = (224, 224, 9)
+    input_shape = (1, *inp)
+    kernel_shape = (3, 3)
+    padding = (1, 1, 1, 1)
+    padding_out = (1, 33, 33, 1)
+
+    @tf.function
+    def simple_net(x):
+        weight_shape = [kernel_shape[0], kernel_shape[1], input_shape[3], 3]
+        weights = tf.constant(np.random.uniform(size=weight_shape), dtype=tf.float32)
+        op = tf.nn.conv2d(
+            x,
+            filters=weights,
+            strides=1,
+            padding="SAME",
+            data_format="NHWC",
+            dilations=1,
+        )
+        op = tf.pad(
+            op,
+            [[0, 0], [padding[0], padding_out[2]], [padding_out[1], padding[3]], [0, 0]],
+            "CONSTANT",
+        )
+        op = tf.pad(
+            op,
+            [[0, 0], [padding[0], padding[2]], [padding[1], padding[3]], [0, 0]],
+            "CONSTANT",
+        )
+        return tf.pad(
+            op,
+            [[0, 0], [padding_out[0], padding[2]], [padding[1], padding_out[3]], [0, 0]],
+            "CONSTANT",
+        )
+
+    from tests.python.contrib.test_ethosu.infra import get_tflite_graph
+
+    mod, params = infra.get_tflite_graph(simple_net, [input_shape])
+
+    operations_distribution = analyze_operations_distribution(mod)
+
+    expected = {
+        "Pad_PART_0": ["generic", "generic", 0],
+        "Conv2D2_PART_2": ["ethos-u", "ethos-u.qnn_conv2d", 1],
+        "Conv2D2_PART_1": ["ethos-u", "ethos-u.qnn_conv2d", 1],
+        "Conv2D2_PART_0": ["ethos-u", "ethos-u.qnn_conv2d", 1],
+        "Identity_PART_0": ["ethos-u", "ethos-u.pad2d", 2],

Review Comment:
   Out of interest, what makes this pad op get called `identity`?



-- 
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] chunit-quic commented on a diff in pull request #13212: [TVMC][microNPU] tvmc option for printing which operators are offloaded to Ethos-U

Posted by "chunit-quic (via GitHub)" <gi...@apache.org>.
chunit-quic commented on code in PR #13212:
URL: https://github.com/apache/tvm/pull/13212#discussion_r1090188874


##########
python/tvm/relay/frontend/common.py:
##########
@@ -1067,6 +1067,20 @@ def __init__(self, span):
             self._span = tvm.relay.Span(tvm.relay.SourceName(span.decode("utf-8")), 0, 0, 0, 0)
         else:
             assert False, f"unsupported span type: {type(span)}"
+        self.suffix_str = "_PART_"
+        self.counter = 0
+        self.distance_from_leaf = -1
+
+    def _create_span(self):
+        """Adds suffix_str + counter value to _span.source_name.name,
+        to create a unique source_name for the Relay layer
+        """
+        if self.distance_from_leaf == 0:
+            return tvm.relay.Span(tvm.relay.SourceName(self._span), 0, 0, 0, 0)
+        self.distance_from_leaf -= 1
+        span_str = "{}{}{}".format(self._span.source_name.name, self.suffix_str, str(self.counter))
+        self.counter += 1
+        return tvm.relay.Span(tvm.relay.SourceName(span_str), 0, 0, 0, 0)

Review Comment:
   > This seems sensible to me, cc the original author of span filling @chunit-quic just to check
   
   Hi all,
   
   Thank you for informing us this change, we just check the PR. We have some ideas about it, and we would like to share some discussions about the suffix in the forum with you.
   
   About the way how span is used in the PR, it looks to me like you would like to take the
   source_name of span as an indicator to the original Relay call(expression). 
   We find the hash value might be enough for your use case rather than making a unique source_name for a span. Please allow me to describe some problems might be cuased by adding suffix.
   
   We removed suffix in the new implementation in our [preRFC](https://discuss.tvm.apache.org/t/pre-rfc-tvm-explorer-infrastructure/13457#reference-level-explanation-9). In short, we found adding suffix might mislead in some cases. Here is an example:
   
   Suppose:
   1. An user thinks span meaning where an expression is from
   2. A conv2d layer is converted to 2 relay exprs (conv2d and relu) in a specific frontend.
   
   The conversion of a simple frontend model with only a conv2d layer will result in this:
   
   ```python
   #FRONTEND
   conv2d layer, named as my_conv2d
   #Relay IR
   %0 = conv2d() /* my_conv2d_part_0 */
   %1 = relu() /* my_conv2d_part_1 */
   ```
   In the case above, that user might assume there are "two layers" in the original frontend model, rather than thinking the converted Relay exprs are from the same forntend layer.
   One is conv2d layer with the name "my_conv2d_part_0", and a relu layer which is named as "my_conv2d_part_1".
   
   Yet with help of suffix we do have some advantages. Like locating the expression which generates the tensor output result of its frontend layer. So far we have [many discussions](https://discuss.tvm.apache.org/t/pre-rfc-tvm-explorer-infrastructure/13457/26?u=chunit) with community but we are still thinking about whether there is a better way to deal with.
   
   Therefore currently we won't recommend you to add suffix to the source_name of span. After checking the codes, perhaps you could consider using the hash value, `tvm.ir.structural_hash()` of an expr as the key of dictionary, like:
   
   ```python
   #python/tvm/relay/analysis/operations_distribution.py
   class AnalyzeOperationsDistribution(ExprVisitor):
       #...
       def visit_call(self, call: relay.Call):
           if isinstance(call.op, tvm.ir.Op):
               self.unique_op_ids[tvm.ir.structural_hash(call)] = [self.compiler_name, self.func_name, self.func_id]
   #python/tvm/driver/tvmc/compiler.py
   def dump_operation_offloads(...):
       def annotate_f(x):
           ret = ""
           if isinstance(x, relay.Call):
               compiler_name, op_name, func_id = operations_distribution[tvm.ir.structural_hash(x)]
               ret = f", func_id: {func_id}, compiler_name: {compiler_name}, op_name: {op_name}"
           return ret
   ```
   
   Because no rewriting/mutation is invoked in `dump_operation_offloads()`, hash value won't be changed. Maybe hash value is an acceptable alternative?
   
   Thank you for reading! Feel free if you have any question. We will try to get back to you soon. :)
   
   



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