I found out that sizes of tiled subviews for convolutions are bigger by 1 than they should. Let’s consider 1D convolution case without batches or channels. Furthermore let m iterate over the output and n over the kernel then input is accessed with m + n. In tiling subview sizes for convolutions are computed by applying requested tile size together with kernel size to the above mentioned expression thus let’s say for tile size of 2 the subview size is 2 + size(n), which is bigger by one as it should since we move kernel only once. I submitted an ugly fix for that but was asked to initiate a discussion here in order to find the best possible solution. I would appreciate any ideas for improvement! Thanks.
Thanks @limo1996 for starting this thread. A bit more detail maybe. The affine maps for the output, filter and input in convolution (i.e linalg.conv) AFAIK are
affine_map[(d0, d1, d2, d3, d4, d5, d6) -> (d0, d1, d2, d3)>]
affine_map[(d0, d1, d2, d3, d4, d5, d6) -> (d3, d4, d5, d6)>]
affine_map[(d0, d1, d2, d3, d4, d5, d6) -> (d0, d4, d2 + d5, d3 + d6)>]
The maps here are used for indexing, so the range of these are [0...,dim(..)-1] . When applied to compute sizes the value substituted is dim(...), which works when the AffineExpr is just an AffineDimExpr. For AffineBinaryOpExpr this becomes incorrect.
The correct solution here seems to be when computing the sizes, instead of just setting the value to dim(..) , we need to set the value to dim(..)-1 and add a +1 the end to get the actual size. This would require some change to the size computation here. Instead of applyMapToValues need to use subViewSizes - 1 and then add 1 to get the final size.
@nicolasvasilache to verify.
Yup it’s just an issue of handling of closed and open intervals.
The range itself is half-open. Before composing, it needs to be turned into a closed interval (i.e. subtract 1 from the upper bound). After the composition, the result needs to be made half-open again (i.e. add 1 to the upper bound).
Ok perfect. Thanks @MaheshRavishankar and @nicolasvasilache . I will change my patch according to your suggestions!