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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
