You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "vacu9708 (via GitHub)" <gi...@apache.org> on 2025/07/06 14:21:47 UTC
[PR] [Relax] Fix a bug that occurred due to shape inference not handling static dim vs symbolic dim [tvm]
vacu9708 opened a new pull request, #18119:
URL: https://github.com/apache/tvm/pull/18119
# Summary
This PR fixes https://github.com/apache/tvm/issues/17964 that occurs through the following process:
1. The Compress node produces a symbolic dimension "num_nonzero"
2. The relax.op.add() in the BiasGelu node adds shape=[2, num_nonzero] and shape=[3]
3. The current shape inference function does not take into account static vs symbolic dimension cases and produces "None" shape, which leads to the error.
# Changes
- Add comments on each case
- Add support for comparing static and symbolic dimensions in shape inference
# Notes
The current shape inference takes into account the following cases
| Case | Error? | Example | expected output dim |
|----------------------------------|---------|-------------------|---|
| static dim(1) | No | (2, 3) + (1, 3) | (2, 3) |
| equal static dims | No | (2, 3) + (2, 3) | (2, 3) |
| equal symbolic dims | No | (n, m) + (n, m) | (n, m) |
However, it does not take into account the following cases
| Case | Error? | Example | expected output dim |
|----------------------------------|---------|-------------------|---|
| static dim vs symbolic dim | Yes | (2, 3) + (2, n) | (2, 3) because the 2nd dim must be 3 regardless of the symbolic dim(n) |
| different symbolic dims | Yes | (2, n) + (2, m) | (2, n) or (2, m) because output dim cannot be determined in compile time |
I was going to add support for the "different symbolic dims" case by introducing a new symbolic dim representing the output dim as follows:
```cpp
// different symbolic dims whose output dim cannot be determined in compile time
static int _bcast_counter = 0;
std::string name = "bcast_dim_" + std::to_string(_bcast_counter++);
PrimExpr new_dim = tir::Var(name, DataType::Int(64));
output_shape.push_back(new_dim);
```
However, the current test code assumes that output dimensions are always derived from input dimensions, so I didn't add support for that case at this time.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Relax] Fix a bug that occurred due to shape inference not handling static dim vs symbolic dim [tvm]
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen closed pull request #18119: [Relax] Fix a bug that occurred due to shape inference not handling static dim vs symbolic dim
URL: https://github.com/apache/tvm/pull/18119
--
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: discuss-archive-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: discuss-archive-unsubscribe@tvm.apache.org
For additional commands, e-mail: discuss-archive-help@tvm.apache.org
Re: [PR] [Relax] Fix a bug that occurred due to shape inference not handling static dim vs symbolic dim [tvm]
Posted by "vacu9708 (via GitHub)" <gi...@apache.org>.
vacu9708 commented on PR #18119:
URL: https://github.com/apache/tvm/pull/18119#issuecomment-3066883199
I found no precedent in the entire source code for asserting or validating dynamic shapes and raising a runtime error.
I’m afraid I won’t be able to implement your suggestion until such a precedent exists.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Relax] Fix a bug that occurred due to shape inference not handling static dim vs symbolic dim [tvm]
Posted by "Hzfengsy (via GitHub)" <gi...@apache.org>.
Hzfengsy commented on PR #18119:
URL: https://github.com/apache/tvm/pull/18119#issuecomment-3043516523
I disagree with the premise that (2, 3) + (2, n) can be statically inferred to have the shape (2, 3). The validity of this operation depends on the runtime value of `n`.
Specifically:
- If `n=1` or `n=3`, the operation is valid due to broadcasting, and the output shape is indeed `(2, 3)`.
- If `n` is any other value, the shapes are incompatible, and the operation should raise a runtime error.
To handle this correctly, we need a mechanism for runtime shape validation. I suggest the following steps:
1. Assume the valid shape: For this specific operation, we can tentatively define the output shape as `(2, 3)` (as this pull request does), since it's the only possible shape for a valid operation.
2. Insert a runtime check: We MUST add an assertion to the model to verify at runtime that `n` is either 1 or 3.
3. (Optional) Hoist the checks: We could write an optimization pass to lift all such runtime assertions to the beginning of the model and potentially combine them for efficiency.
cc @tqchen
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Relax] Fix a bug that occurred due to shape inference not handling static dim vs symbolic dim [tvm]
Posted by "vacu9708 (via GitHub)" <gi...@apache.org>.
vacu9708 commented on PR #18119:
URL: https://github.com/apache/tvm/pull/18119#issuecomment-3043680867
@Hzfengsy Thanks for your feedback.
I'll review the mechanism you proposed, try to implement it, and get back to you.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org