Skip to content

Depthwise conv uses 4d filter, fixed wrong 3d assumption. #9732

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
copybara-service merged 1 commit into from
Mar 27, 2026
Merged

Depthwise conv uses 4d filter, fixed wrong 3d assumption. #9732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion test/subgraph/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,6 @@ xnnpack_unit_test(
xnnpack_unit_test(
name = "depthwise_convolution_2d_test",
srcs = ["depthwise-convolution-2d.cc"],
tags = ["no_ynnpack"],
deps = SUBGRAPH_TEST_DEPS + [":stencil"],
)

Expand Down
7 changes: 4 additions & 3 deletions test/subgraph/depthwise-convolution-2d.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ Tensor<float> ReferenceImpl(Tensor<Data> input, Tensor<Filter> filter,
float padded_nyxc =
dequantize(padded(n, y, dy, x, dx, ic), input_quantization);
float filter_yxc =
dequantize(filter(dy, dx, c), filter_quantization);
dequantize(filter(0, dy, dx, c), filter_quantization);
output_nyxc += padded_nyxc * filter_yxc;
}
}
Expand Down Expand Up @@ -167,6 +167,7 @@ void TestImpl(bool channelwise_quantization = false) {

// Make a random filter.
std::vector<size_t> filter_shape = {
1,
params.kernel.height,
params.kernel.width,
params.input_channels * params.depth_multiplier,
Expand Down Expand Up @@ -195,7 +196,7 @@ void TestImpl(bool channelwise_quantization = false) {
xnn_quantization_params filter_quantization =
random_quantization(xnn_datatype_of<Filter>(), rng, 0.001f, 2.0f);
Tensor<float> filter_scale(
{channelwise_quantization ? filter.extent(2) : 1});
{channelwise_quantization ? filter.extent(3) : 1});
if (filter_scale.size() > 1) {
// Generate random per-channel scales, in the range of the original scale.
std::uniform_real_distribution<float> filter_scale_dist(
Expand Down Expand Up @@ -235,7 +236,7 @@ void TestImpl(bool channelwise_quantization = false) {
input_id);
if (channelwise_quantization) {
subgraph.AddStaticTensorQS8(
filter.extents(), /*channel_dim=*/2, TensorType::kDense,
filter.extents(), /*channel_dim=*/3, TensorType::kDense,
filter_scale.data(), filter_id,
/*flags=*/0, reinterpret_cast<int8_t*>(filter.base()));
} else {
Expand Down
40 changes: 26 additions & 14 deletions ynnpack/xnnpack/subgraph.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ xnn_status xnn_define_convolution_2d(
// close: first two dims are the same between filter and output, except
// that output has extra dimension of extent 1).
const size_t filter_scale_split[] = {groups, 1, group_output_channels};
status = ynn_define_split_dim(subgraph->ynn, 0, 3, filter_scale_split,
status = ynn_define_split_dim(subgraph->ynn, -1, 3, filter_scale_split,
filter_scale_id, &split_id, /*flags=*/0);
if (status != ynn_status_success) {
return ynn::xnn_status_from_ynn(status);
Expand Down Expand Up @@ -390,33 +390,45 @@ xnn_status xnn_define_depthwise_convolution_2d(

// We need to transpose and expand a filter buffer, so it matches format
// expected by the grouped convolution.
// [kh, kw, ci * dm] -> [ci * dm, kh, kw]
int32_t swap_dims[3] = {2, 0, 1};
// [1, kh, kw, ci * dm] -> [ci * dm, kh, kw, 1]
int32_t swap_dims[4] = {3, 1, 2, 0};

status = ynn_define_static_transpose(subgraph->ynn, 3, swap_dims, filter_id,
status = ynn_define_static_transpose(subgraph->ynn, 4, swap_dims, filter_id,
&transposed_filter_id, /*flags=*/0);

if (status != ynn_status_success) {
return ynn::xnn_status_from_ynn(status);
}

uint32_t expanded_transposed_filter_id = YNN_INVALID_VALUE_ID;
// [kh, kw, ci * dm] -> [ci * dm, kh, kw, 1]
int32_t new_axes[] = {3};
status = ynn_define_static_expand_dims(subgraph->ynn, 1, new_axes,
transposed_filter_id,
&expanded_transposed_filter_id,
/*flags=*/0);
if (status != ynn_status_success) {
return ynn::xnn_status_from_ynn(status);
uint32_t old_scale_id = subgraph->ynn->value(transposed_filter_id).scale_id;

// Depthwise convolution has incompatible quantization dims to other ops, thus
// new scale is needed for channelwise quantized variants.
if (old_scale_id != YNN_INVALID_VALUE_ID &&
ynn::rank_of_value(subgraph->ynn, old_scale_id) >= 1) {
uint32_t new_scale_id = YNN_INVALID_VALUE_ID;

size_t quantization_dims[] = {1, 1, 1, depth_multiplier * input_channels};

// XNNPACK copies the scale data from the caller, do the same here.
const uint32_t scale_flags = YNN_VALUE_FLAG_COPY_DATA;
status = ynn_define_tensor(subgraph->ynn, ynn_type_fp32, 4,
quantization_dims,
subgraph->ynn->value(old_scale_id).data->base,
scale_flags, &new_scale_id);
if (status != ynn_status_success) {
return ynn::xnn_status_from_ynn(status);
}

subgraph->ynn->value(transposed_filter_id).scale_id = new_scale_id;
}

return xnn_define_convolution_2d(
subgraph, input_padding_top, input_padding_right, input_padding_bottom,
input_padding_left, kernel_height, kernel_width, subsampling_height,
subsampling_width, dilation_height, dilation_width, input_channels,
/*group_input_channels=*/1, depth_multiplier, output_min, output_max,
input_id, expanded_transposed_filter_id, bias_id, output_id, flags);
input_id, transposed_filter_id, bias_id, output_id, flags);
}

xnn_status xnn_define_space_to_depth_2d(xnn_subgraph_t subgraph,
Expand Down
Loading