rafaqz / Stencils.jl

Static Stencil mapping for Julia
Other
8 stars 2 forks source link

Strange behaviour with SwitchingStencilArray #45

Closed asinghvi17 closed 3 days ago

asinghvi17 commented 6 months ago

When using a SwitchedStencilArray with either a Raster or a regular Array, under mapstencil!, I get extremely strange output. mapstencil seems to be fine.

Example:

using Rasters, Stencils, TestImages
using CairoMakie

lena = testimage("lena") .|> CairoMakie.Colors.Gray .|> Float64

heatmap(lena)

nabla = Stencils.Kernel(Stencils.Cross(1, 2), [0.5, 0.5, 0, 0.5, 0.5])

sa = StencilArray(lena, nabla)
mapstencil(kernelproduct, sa) |> heatmap # this is OK

ra = Raster(lena; dims = (X(1:size(lena, 1)), Y(1:size(lena, 2))))

mapstencil(kernelproduct, StencilArray(ra, nabla)) |> heatmap # this is OK
mapstencil(kernelproduct, SwitchingStencilArray(ra, nabla)) |> heatmap # this is OK
mapstencil!(kernelproduct, SwitchingStencilArray(ra, nabla)) |> x -> heatmap(x.source) # BAD BAD BAD
mapstencil!(kernelproduct, SwitchingStencilArray(lena, nabla)) |> x -> heatmap(x.source) # BAD BAD BAD

download-15

rafaqz commented 6 months ago

Does it need .source?

Probably .dest has the data, and you should just plot the output.

The return value should be the new switched array. Its not mutable. ah right you have that

(I only use the abstract type so it may also be broken)

asinghvi17 commented 6 months ago

Source and dest are the same here...so it may actually be a broken constructor or something. Will have a look...

asinghvi17 commented 3 days ago

As far as I can tell, this looks correct now. Potentially a consequence of the DiskArrays PR I made for collect_similar.

rafaqz commented 3 days ago

There are other major problems with SwitchingStencilArray, I'm fixing it currently

rafaqz commented 3 days ago

I only use it as an abstract type through DynamicGrids and the one here is not well tested