AndrewZhaoLuo commented on code in PR #13011:
URL: https://github.com/apache/tvm/pull/13011#discussion_r990541699
##########
python/tvm/relay/frontend/onnx.py:
##########
@@ -5654,10 +5657,20 @@ def _parse_graph_input(self, graph):
)
warnings.warn(warning_msg)
if isinstance(self._dtype, dict):
- dtype = self._dtype[i_name] if i_name in self._dtype else
d_type
+ dtype = (
+ self._dtype[i_name_compatible]
+ if i_name_compatible in self._dtype
+ else d_type
+ )
else:
dtype = d_type
- self._nodes[i_name] = new_var(i_name, shape=i_shape,
dtype=dtype)
+ self._nodes[i_name_compatible] = new_var(
+ i_name_compatible, shape=i_shape, dtype=dtype
+ )
+
+ if i_name_compatible:
Review Comment:
In it's current state it seems the intention this branch will always be
evaluated in the else branch on line 5643.
##########
python/tvm/relay/frontend/onnx.py:
##########
@@ -5642,9 +5643,11 @@ def _parse_graph_input(self, graph):
continue
else:
Review Comment:
I notice you only handle the else branch here. Will we not have the same
parsing error from the other branches?
##########
python/tvm/relay/frontend/onnx.py:
##########
@@ -5629,6 +5629,7 @@ def _parse_graph_initializers(self, graph):
def _parse_graph_input(self, graph):
for i in graph.input:
+ i_name_compatible = None
Review Comment:
this seems a bit messy as now i_name_compatible can be of types [None, str].
This is actually an issue since the python str of "" is not truthy (it is
false) which may be able to be an onnx name (correct me if im wrong). If this
is the intention it is a bit unclear and deserves a comment.
Why can't we just do something like:
```
new_name = i.name.replace(":", "_")
self._renames[i.name] = new_name
i.name = new_name
```
--
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]
