I attached a patch with the correct implementation. The correct result for convn(vvv, vvv, 'valid') should be vvvv(1,1,2) that is 4 instead we get the same result as we computed dot(v, v). The following octave example is straightforward: The 'b + mb*jb' argument should be 'b + mb*(nb-jb-1)' in fact convolution is always a sum of products of this type: a(u1, u2, u3) * b(x1-u1, x2-u2, x3-u3). The bug is inside the convolve_nd function (liboctave/numeric/) and exactly inside the following loops:įor (octave_idx_type ja = 0 ja (a + ma*(ja + jb), ad, acd, b + mb*jb, bd, bcd, Indeed the problem can be reproduced with integer values too and is present only for arrays with more than 2 dimensions. Here is it again.Ĭompare the following before and after the patch:ĭ = full(3:10,3:10,3:10) - valid # these should be exactly the same I see now that I messed up the markup on comment #7 so half of the the comment is gone. cset 7903ae3b657d 'convn: fix incorrect tests' only fixed the tests that were wrong.īut the bug about precision as mentioned on comment #7 still holds and that's why the tests are failing (this is not a regression, the tests are failing because the tests are new but the bug has always been there). When I said on comment #9 'Already ahead of you :) those are already fixed' I meant the failing tests due to wrong array dimensions mentioned on comment #8.

This would avoid having a separate routine for the valid shape, and possibly speed-up the shape 'same' which is currently implemented as computing the 'full' shape and then clipping the borders. I may be recalling wrong, but I think simply adding an extra offset when passing the pointers to convolve_nd should be enough. I'm unsure why reverting it was required. Summary: perform convolution properly independently of matrix orientation (bug #34893)

If the fix is indeed to re-reverse the order of the summation, then the fix may involve reverting the following changesets:

Gnu octave array of functions