padreofthegame commented on issue #12400: URL: https://github.com/apache/tvm/issues/12400#issuecomment-1219137262
Thanks @ganler for your quick and detailed response. As I already said I am new here and I am really interested in the work you did, and you are doing at the moment, and I would like to help if it is possible. So I decided to chose some package to look at, try some features and comment problems that I came up working with it (actually problems may appear due to misunderstanding of some parameters). But here I found some features really interesting: > If axis is [] it means none of the axies will be squeezed so that it is fairly correct to have identical output shapes. I also thought logically about that and in a same way, but then I came up with errors described in a Scenario 2. and 3. of the first Test. Looking at Scenario 2. basically if we think that `axis=[]` does nothing to input tensor (as it is shown from result in Scenario 1.) there should be no problems for example to squeeze that tensor again after first squeeze. But here things becomes to be interesting. From first error in Scenario 2. we see that it pass pre-compile check in python (since `0` is valid dimension to squeeze) but later raise an error that corresponding dimension is not equal to 1. The same thing happens if instead of `0` we use `1, 2` or `3`. Also, if we use axis to be `4` or `6`, which also pass the pre-compile check, we got the error described second in Scenario 2. that we are indexing out of bounds of the tensor. From this, the logical explanation I came up with is that internally `axis=[]` changes shape of the input tensor as it was `None`, because if we assume that output tensor of first squeeze would be `[2, 3, 4, 5] all errors described above should be expected. And this is the main reason I decided to write here. On the other hand I also wrote about feature I found working with parameter `axis` of type Constant because it occur in the same function. In my opinion case `Constant(0)` should be covered because Constant is a valid axis type, and `0` should be valid axis dimension. Also looking deeply in transform package I also find that for example `full` function allow similar types for parameter `shape` as `squeeze` for `axis`, but also cover the case when the `shape` is `int` for example. What is your opinion about that since you are much longer in this community? If needed, I could go through similar functions in the package, and try to identify potentially similar features. Maybe, also adding possibility for `axis` to be of type `int` should be good thing, since very often only one dimension should be squeezed? I also provided and tested solution I mentioned above, and it looks like it eliminate errors mentioned. I would be pleased to push the solution to repository, but at first I wanted to comment with someone relevant about this. -- 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]
