Support broadcast for bias in grouped conv fwd#1081
Conversation
amberhassaan
left a comment
amberhassaan
left a comment
There was a problem hiding this comment.
Looks good from user interface perspective.
| K, Do * Ho * Wo * G * K, 1, Ho * Wo * G * K, Wo * G * K, G * K}; | ||
| // Logical broadcast bias (we have to pass bias lengths in the same format as output - GNKDHW) | ||
| std::array<ck::index_t, 6> bias_lengths{G, 1, K, 1, 1, 1}; | ||
| std::array<ck::index_t, 6> bias_strides{K, 0, 1, 0, 0, 0}; |
There was a problem hiding this comment.
not super important but I think the remaining strides should be >= K to be a valid set of strides.
There was a problem hiding this comment.
Lengths are set to 1 so these strides are also valid. Reference implementation uses these strides to perform logical broadcast.
There was a problem hiding this comment.
For packed case, valid strides wouldn't have 0s. They'd be {K, K, 1, K , K, K}. For non-packed case, they can be anything >= K really.
There was a problem hiding this comment.
If You would like to perform logical broadcast You have to jump to the same memory in each step.
For example:
You would like to use broadcasted memory 3x4x2, where (4x2 is broadcasted 3 times). So the step for the first dimension is 0.
data[0, x, y] == data[1, x, y] == data[2, x, y]
|
|
||
| // | ||
| using G_K = ck::tensor_layout::convolution::G_K; | ||
| using GK = ck::tensor_layout::convolution::G_K; |
There was a problem hiding this comment.
Seems like G_K is a duplicate of GK?
There was a problem hiding this comment.
Thanks. I removed GK. G_K is used in a lot of examples so to keep backward compatibility I left G_K (which is also less intuitive for me). I suppose, it was designed to supported not packed (G_K ) in different layout than packed (GK). At now the both cases are supported using G_K.
| using InLayout = ck::tensor_layout::convolution::NDHWGC; | ||
| using WeiLayout = ck::tensor_layout::convolution::GKZYXC; | ||
| using OutLayout = ck::tensor_layout::convolution::NDHWGK; | ||
| using BiasLayout = ck::tensor_layout::convolution::G_K; |
There was a problem hiding this comment.
one advantage of using a separate layout is that I can tell which one is bias and which one is z when passing as a tuple.
* Support broadcast for bias in grouped conv fwd * Fix comment * Comment fixes * Remove GK layout
4 participants
#1076