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

padarray inconsistent behavior for n=1 axes #259

Open
andrew-saydjari opened this issue May 30, 2023 · 5 comments
Open

padarray inconsistent behavior for n=1 axes #259

andrew-saydjari opened this issue May 30, 2023 · 5 comments

Comments

@andrew-saydjari
Copy link

I found the following surprising.

n = 0
x = rand(50,100);
xr = repeat(x,outer=[1 1 n])
println(size(xr))
pxr = ImageFiltering.padarray(xr,ImageFiltering.Pad(:reflect,(5,5,0)));
println(size(pxr))

For n!=1 this succeeds and pads the repeated matrix which is of dimension (50,100,n) to be (60,110,n). For n=1, this fails with the following error message.

"DimensionMismatch: supplied axes do not agree with the size of the array (got size (1,) for the array and (0,) for the indices"

Maybe there is something I am not understanding, but I would have expected the n=1 case to behave similarly.

@andrew-saydjari
Copy link
Author

A similar problem just came up in another code base in a fairly different application. The handling of zero padding on singleton dimensions seems require (sometimes difficult) special handling, especially if you need shape stability.

ImageFiltering.padarray(zeros(1,100),ImageFiltering.Pad(:reflect,(0,1)))

@andrew-saydjari
Copy link
Author

The following seems to be a fix. Not sure why .= has different behavior than copy!(). Does someone who understands the code base here better know why that is different and if the change to .= below would be acceptable/worth a PR?

using ImageFiltering: AbstractBorder

padarray_fix(img::AbstractArray, border::AbstractBorder) = padarray_fix(eltype(img), img, border)
function padarray_fix(::Type{T}, img::AbstractArray, border) where {T}
    ba = BorderArray(img, border)
    out = similar(ba, T, axes(ba))
    out .= ba
end

@mkitti
Copy link
Member

mkitti commented Jun 29, 2023

If you could write out a few simple test cases, that would be great.

@andrew-saydjari
Copy link
Author

Ok, will do. I just wasn't sure if there was intended behavioral differences between copy! and .=, happy to try to do a few test cases in few days.

@mkitti
Copy link
Member

mkitti commented Jun 29, 2023

I'm not sure if I fully understand the issue yet either, but the test cases would help confirm that we have a fix is in place and it stays fixed.

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

No branches or pull requests

2 participants