junrushao commented on code in PR #13630:
URL: https://github.com/apache/tvm/pull/13630#discussion_r1051176207
##########
python/tvm/script/parser/core/parser.py:
##########
@@ -150,7 +150,7 @@ def add(self, var: str, value: Any, allow_shadowing: bool =
False):
The options of whether variable shadowing allwed for this variable.
"""
# Skip if the key and value are equal to those in the var_table
- if self.name2value[var] and self.name2value[var][-1] == value:
+ if self.name2value[var] and self.name2value[var][-1] is value:
return
Review Comment:
I do not have particular opinion on which style we should go for, but just
wanted to point out the implication of the switch:
- `is` checks referential/pointer equality, or in python's term, identity,
where it returns True only when `id(lhs) == id(rhs)`. It could depend on
certain underlying implementation of the system, for example:
```python
>>> a = 257
>>> b = 257
>>> a is b
False
>>> a = 256
>>> b = 256
>>> a is b
True
```
- `==` checks equality that could be potentially overloaded, for TVM
objects, it's using TVM's address comparison `.same_as()` rather than python's
builtin `id()` method which is used in `is` operator. However
The implication of switching from `==` to `is` means that it bypasses TVM's
`.same_as()` method, which at the moment I am not quite certain is suitable for
broad usecases.
Therefore, how about we do the following: if `lhs` and `rhs` are numpy
arrays, then we use numpy-specific behavior (e.g. elementwise equality), but
otherwise we still use `==`.
--
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]
