gemini-code-assist[bot] commented on code in PR #19521:
URL: https://github.com/apache/tvm/pull/19521#discussion_r3211060553
##########
python/tvm/relax/frontend/tflite/tflite_frontend.py:
##########
@@ -992,11 +995,18 @@ def get_scalar_value(tensor):
if isinstance(expr, relax.Constant):
value = expr.data.numpy()
else:
- # relax.op.arange currently expects scalar-like values
here.
- # Keep dynamic scalar RANGE explicit until frontend
support is added.
- raise tvm.error.OpNotImplemented(
- "TFLite RANGE with dynamic scalar inputs is not
supported in Relax frontend yet."
- )
+ scalar_dtype =
self.get_tensor_type_str(tensor.tensor.Type())
+ if scalar_dtype not in ("int32", "int64"):
+ raise tvm.error.OpNotImplemented(
+ "TFLite RANGE with dynamic non-integer scalar
inputs is not supported in Relax frontend yet."
+ )
+ expr = self.bb.match_cast(expr, relax.TensorStructInfo([],
scalar_dtype))
+ expr = self.bb.normalize(relax.op.astype(expr, "int64"))
+ expr = self.bb.normalize(relax.op.reshape(expr, [1]))
+ shape_expr = self.bb.emit(relax.op.tensor_to_shape(expr))
+ scalar_var = tirx.Var(f"range_{tensor.tensor.Name()}",
"int64")
Review Comment:
The `tensor.tensor.Name()` method returns a `bytes` object, which can lead
to improperly formatted variable names like `range_b'my_name'`. It's better to
use `self.get_tensor_name(self.subgraph, tensor.tensor_idx)` which correctly
decodes the name and provides a fallback if the name is not present.
```suggestion
scalar_var =
tirx.Var(f"range_{self.get_tensor_name(self.subgraph, tensor.tensor_idx)}",
"int64")
```
##########
python/tvm/relax/frontend/tflite/tflite_frontend.py:
##########
@@ -1229,6 +1266,38 @@ def quantize(x):
return out
+ def convert_unique(self, op):
+ """Convert TFLite UNIQUE."""
+
+ from tflite.BuiltinOptions import BuiltinOptions
+ from tflite.TensorType import TensorType
+ from tflite.UniqueOptions import UniqueOptions
+
+ input_tensors = self.get_input_tensors(op)
+ assert len(input_tensors) == 1, "input tensors length should be 1"
+ input_tensor = input_tensors[0]
+ in_expr = self.get_expr(input_tensor.tensor_idx)
+
+ assert op.BuiltinOptionsType() == BuiltinOptions.UniqueOptions
+ op_options = op.BuiltinOptions()
+ unique_options = UniqueOptions()
+ unique_options.Init(op_options.Bytes, op_options.Pos)
+
+ output_tensors = self.get_output_tensors(op)
+ assert len(output_tensors) == 2, "output tensors length should be 2"
+
+ unique_out = relax.op.unique(in_expr, sorted=False, return_index=True)
+ values = relax.TupleGetItem(unique_out, 0)
+ indices = relax.TupleGetItem(unique_out, 1)
+
+ idx_out_type = unique_options.IdxOutType()
+ if idx_out_type == TensorType.INT64:
+ indices = relax.op.astype(indices, "int64")
+ else:
+ indices = relax.op.astype(indices, "int32")
Review Comment:
There are a couple of issues in this function:
1. The `relax.op.unique` operator does not have a `return_index` parameter.
The correct parameter to get the inverse indices is `return_inverse`.
2. The `relax.op.unique` operator returns indices as `int64`. The current
logic performs an unnecessary `astype` operation when the desired output type
is also `int64`. This can be simplified to only cast when the output type is
`int32`.
```suggestion
unique_out = relax.op.unique(in_expr, sorted=False,
return_inverse=True)
values = relax.TupleGetItem(unique_out, 0)
indices = relax.TupleGetItem(unique_out, 1)
idx_out_type = unique_options.IdxOutType()
if idx_out_type == TensorType.INT32:
indices = relax.op.astype(indices, "int32")
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
