Skip to content
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.

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

Incorrect output from conv2d deep kernel function #4

Closed
keithm-xmos opened this issue Oct 14, 2020 · 1 comment · Fixed by #7
Closed

Incorrect output from conv2d deep kernel function #4

keithm-xmos opened this issue Oct 14, 2020 · 1 comment · Fixed by #7
Assignees

Comments

@keithm-xmos
Copy link
Contributor

The following integration test config produces incorrect output values for the 2nd output channel group.

NOTE: This test passes when running on the x86 host.

integration_test/test_single_op_models/test_conv2d/test_explicit_padding/test_padded_conv2d.py::test_output[CONFIGS[0]]
  0:
    K_h: 9
    K_w: 7
    height: 8
    input_channels: 32
    num_threads: 1
    output_channels: 28
    pad_b: 2
    pad_l: 1
    pad_r: 0
    pad_t: 0
    strides:
    - 2
    - 2
    width: 7

Predicted and expected output values:

idx=(0, 0, 16): diff=117, expected=10, predicted=127
idx=(0, 0, 17): diff=141, expected=-14, predicted=127
idx=(0, 0, 18): diff=-51, expected=58, predicted=7
idx=(0, 0, 19): diff=113, expected=14, predicted=127
idx=(0, 0, 20): diff=51, expected=76, predicted=127
idx=(0, 0, 21): diff=106, expected=21, predicted=127
idx=(0, 0, 22): diff=157, expected=-30, predicted=127
idx=(0, 0, 23): diff=62, expected=51, predicted=113
idx=(0, 0, 24): diff=-104, expected=115, predicted=11
idx=(0, 0, 25): diff=-199, expected=71, predicted=-128
idx=(0, 0, 26): diff=126, expected=1, predicted=127
idx=(0, 0, 27): diff=-141, expected=13, predicted=-128
@keithm-xmos keithm-xmos transferred this issue from xmos/ai_tools Dec 15, 2020
@keithm-xmos
Copy link
Contributor Author

From @astewart-xmos

I'm realizing now that I'm not sure I understand the parameters listed in that ticket.

It says the width (presumably the input width?) is 7. The kernel width is 7, the left padding is 1, and the horizontal stride is 2.
Does that mean that the output width is only 1 pixel?

Same with the output height. height = 8, K_h = 9, pad_t = 0 ---> These suggest that for the first output pixel, the bottom row of the convolution window will be in the padding below the input image.

But then, pad_b = 2 and vertical stride = 2. If the output height is 1 pixel, then the second row of padding on the bottom is unnecessary (i.e. pad_b should be 1). If the output height is 2 pixels, then then for the second row of output pixels, the bottom row of the convolution window is below the padding ( i.e. pad_b should be 3)

astewart-xmos pushed a commit that referenced this issue Jan 14, 2021
…tail_deep_padded().

The assembly implementations of those two functions misbehaved under certain conditions. In particular, the conditions seem to be that the input image has no input channel tail (channel count is multiple of 32), and there are multiple non-padding columns within the convolution window. It seems likely that there are additional conditions that would need to have been met, because otherwise I expect this would have manifested sooner.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants