[Relax][Frontend][TFLite] Fix dynamic FILL/SPLIT_V partial implementations#19433
Conversation
There was a problem hiding this comment.
Code Review
This pull request enhances the TFLite frontend by enabling support for dynamic dimensions in the Fill operator and dynamic split sizes in the SPLIT_V operator. The implementation utilizes tensor_to_shape for dynamic Fill operations and decomposes dynamic SPLIT_V operations into a series of dynamic strided slices. Reviewer feedback recommends renaming the variables num_splits and rank to num_outputs and input_rank, respectively, to enhance code clarity.
| padded_cumsum = relax.op.concat([zero, cumsum], axis=0) | ||
| # TFLite fixes the tuple arity in the graph, even when the split | ||
| # sizes themselves are supplied at runtime. | ||
| num_splits = len(output_tensors) |
|
|
||
| # end_base is the full input shape; only split_axis changes per slice. | ||
| end_base = relax.op.shape_to_tensor(relax.op.shape_of(in_expr)) | ||
| begin_base = relax.const(np.zeros((rank,), dtype="int64"), "int64") |
There was a problem hiding this comment.
Thanks, these are naming-only suggestions. Since the variables are local and the current intent is already documented by comments, I’d prefer to keep the diff minimal.
tlopex
left a comment
There was a problem hiding this comment.
FILL frontend looks good, but dynamic FILL still breaks at legalize.
The issue is in relax.full legalize (legalize_ops/create.py): it assumes call.args[0] is a static ShapeExpr and passes it directly to topi.full. For dynamic FILL, this can be a tensor_to_shape(...) call, so legalize fails before tvm.compile.
Could you have a fix for _full to handle dynamic shape args just in this PR dynamic_strided_slice in legalize_ops/index.py should be a good template: keep the current static ShapeExpr path, and for dynamic ShapeStructInfo, match-cast to fresh symbolic vars and pass those vars to topi.full.
Then we can update test_fill_dynamic_dims to actually run the verify test
| assert "R.scatter_elements" in ir | ||
|
|
||
| # Regression guard: compile to catch the dynamic split lowering path. | ||
| tvm.compile(mod, tvm.target.Target("llvm")) |
There was a problem hiding this comment.
Is this really needed? I meantvm.complie here
There was a problem hiding this comment.
You're right, adding tvm.compile(...) to test_split_v_dynamic was over-testing on my side.
That test is meant to cover the frontend lowering change for dynamic SPLIT_V, namely preserving fixed tuple arity and lowering to the expected Relax ops. Those
properties are already directly captured by the IR assertions, so I removed the extra tvm.compile(...) / verify(...) from that test.
For dynamic FILL, I kept the compile check intentionally. In that case, the imported Relax IR was already shape-aware, while the actual remaining issue was that
dynamic-shape relax.full still failed in the legalize/compile path. So I updated the legalization for dynamic relax.full, and kept tvm.compile(...) in
test_fill_dynamic_dims to cover that specific regression.
…ze_splits in SPLIT_V This PR fixes partial implementations for FILL and SPLIT_V operators. FILL: - Remove OpNotImplemented when dims is non-constant - Use relax.op.tensor_to_shape to convert dynamic dims tensor to shape - Pass the shape expression to relax.op.full SPLIT_V: - Remove OpNotImplemented when size_splits is non-constant - Decompose dynamic SPLIT_V into multiple dynamic_strided_slice ops - Use cumsum to compute split offsets and scatter_elements to build begin/end tensors for each slice Tests: - Add test_fill_dynamic_dims - Add test_split_v_dynamic
c043efc to
34093e8
Compare
|
Addressed both review points:
One extra fix was needed in addition to _full: LegalizeOps was filtering out dynamic relax.full before the legalization hook could run, so I also relaxed the op’s shape gate to make the dynamic legalization path reachable. After that, dynamic FILL legalizes and compiles correctly. |
|
The imported Relax IR was not well formed before legalization. I fixed the frontend to bind runtime fill dims to fresh shape vars before calling relax.full |
|
Please take a look @tlopex |
… `STRIDED_SLICE/SPLIT_V` tests (#19468) ## Summary This PR continues the TFLite frontend work tracked in #18971 for `STRIDED_SLICE` and `SPLIT_V`. Since the dynamic `FILL` / `SPLIT_V` partial-implementation work has already been handled separately in #19433, this PR focuses on the remaining pieces in this branch: - fixing negative-stride `STRIDED_SLICE` conversion in the TFLite frontend - adding regression coverage for `STRIDED_SLICE` and static `SPLIT_V` Relates to #18971. ## Changes 1. **`STRIDED_SLICE` negative-stride fix** - Update the TFLite frontend `convert_strided_slice` handling of `end_mask` when `stride < 0`. - Use an exclusive lower bound compatible with Relax slicing semantics so reverse slices like `x[::-1]` include index `0` correctly. 2. **TFLite frontend test coverage** - Add `test_strided_slice_stride` to cover non-unit stride handling. - Add `test_strided_slice_negative_stride` to cover reverse slicing with negative strides. - Add `test_split_v_static` to cover static `SPLIT_V` conversion. 3. **Scope clarification** - Keep this PR focused on the remaining `STRIDED_SLICE` / `SPLIT_V` work from #18971. - Exclude the dynamic `FILL` / `SPLIT_V` changes that are already addressed in #19433. ## Testing ```bash python -m pytest -n 1 tests/python/relax/test_frontend_tflite.py::test_split_v_static -q python -m pytest -n 1 tests/python/relax/test_frontend_tflite.py::test_strided_slice_stride -q python -m pytest -n 1 tests/python/relax/test_frontend_tflite.py::test_strided_slice_negative_stride -q ``` ## Result - The added STRIDED_SLICE and SPLIT_V tests passed locally. - The negative-stride STRIDED_SLICE path now matches Relax slicing semantics.
This PR fixes partial TFLite frontend support for dynamic
FILLandSPLIT_V.Key changes:
FILLto accept runtimedimstensors by converting them withrelax.op.tensor_to_shapebefore callingrelax.op.full.SPLIT_Vto accept runtimesize_splitstensors by decomposing theop into
cumsumanddynamic_strided_slice.SPLIT_V, instead of relying onsize_splitsstatic shape information.FILLimport anddynamic
SPLIT_Vimport/compile behavior.This addresses the
FILLandSPLIT_Vitems under the "Fix partialimplementations" section of #19412.
Validation:
python -m pytest tests/python/relax/test_frontend_tflite.py \ -k "split_v_dynamic or fill_dynamic_dims" -vResult: