hiyufan commented on PR #20255:
URL: https://github.com/apache/tvm/pull/20255#issuecomment-5534628534
Confirmed and fixed in `d9b542d`. You are right, and the wrong answer is not
even empty:
```
(batch, 0, 4).reshape(0, 4) torch (0, 4) before: (s77, 4)
```
The guard was `if None in current or 0 not in current: return [dims]`, and
the `None in current` half is simply wrong reasoning on my part. I wrote it to
mean "I cannot see the whole shape, so stay out of the way", but a single
statically known zero already fixes the element count at zero whatever the
symbols turn out to be — the other dimensions do not need to be known for that.
Now the guard asks only for a known zero.
The symbolic dimensions do still matter one level down, where a position
that needs holding is filled in. A literal there would be re-read as a copy, so
the position is held as the expression it already is, which is not a literal
and so is not substituted; a later step rewrites it once it has become a real
zero.
I also removed a no-op the loop was emitting: after a rewrite it appended
the target once more, so your case lowered to two reshapes. Once nothing needs
rewriting the previous step has already produced the target, so the extra one
is only emitted when no rewrite happened at all. Your case is a single reshape
now:
```python
lv: R.Tensor((0, 4), dtype="float32") = R.reshape(x, R.shape([0, 4]))
```
### Verification
I swept it rather than checking your example alone, since hand-picked cases
are what let the previous hole through — a case that passed by coincidence read
as confirmation. 7 input layouts mixing symbolic and static dims around a known
zero, against 9 targets, compared to torch:
```
after 63 compared 63 matched 0 mismatched
before 63 compared 27 matched 36 mismatched
(3,0,4) dyn=(0,) target=(0,4) torch=(0, 4) tvm=('s13', 4)
(3,0,4) dyn=(0,) target=(0,) torch=(0,) tvm=('s13',)
(3,0,4) dyn=(0,) target=(0,0,4) torch=(0, 0, 4) tvm=('s13', 0, 4)
```
The 2132-case static sweep is unchanged at zero mismatches, so the symbolic
path did not cost the static one anything.
### Test
`test_reshape_zero_sized_dim_dynamic_batch`, using `Dim("batch", min=1,
max=64)` over a `(3, 0, 4)` example. It fails on the previous head of this PR
and passes now. `verify_model_numerically` gained a `dynamic_shapes`
passthrough to carry it — the expected-IR form is not available here for the
reason in the PR description: re-parsing `R.reshape(x, R.shape([0, 4]))`
applies the copy rule again, which is the same round-trip gap an `allowzero` on
`relax.op.reshape` would close.
Full suites: 24 failed / 417 passed against 24 failed / 412 passed on clean
`main`; the 24 are pre-existing in my environment and identical either way.
`ruff format --check` and `ruff check` clean.
--
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]
