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

Adds optimised algorithm for Median Filter #34

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
189 changes: 184 additions & 5 deletions src/mapwindow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,19 @@ end

mapwindow(f, img, window::AbstractArray, args...; kwargs...) = mapwindow(f, img, (window...,), args...; kwargs...)



function mapwindow{T,N}(f,
img::AbstractArray{T,N},
window::Indices{N},
border::BorderSpecAny;
callmode=:copy!)
_mapwindow(replace_function(f), img, window, border, default_shape(f); callmode=callmode)
if(is_medianfilter(f))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of introducing is_medianfilter, a more "Julian" approach would be to change the signature of your median_filter function below to be the following method:

function mapwindow{T,N}(f::typeof(median!),

But I actually have a slightly different suggestion, see below.

median_filter(replace_function(f), img, window, border, default_shape(f); callmode=callmode)
else
_mapwindow(replace_function(f), img, window, border, default_shape(f); callmode=callmode)
end

end
function _mapwindow{T,N}(f,
img::AbstractArray{T,N},
Expand Down Expand Up @@ -116,6 +123,89 @@ function _mapwindow{T,N}(f,
out
end



# This is a implementation of Fast 2D median filter
# http://ieeexplore.ieee.org/document/1163188/
function median_filter{T,N}(f,
img::AbstractArray{T,N},
window::Indices{N},
border::BorderSpecAny,
shape=default_shape(f);
callmode=:copy!)
inds = indices(img)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the original algorithm is faster for smaller window sizes, you could insert something like this:

if prod(map(length, window)) < 81
    return _mapwindow!(...)
end

and keep the original implementation as well.

However, this makes testing a little harder: you'd like to check, for example, that the two algorithms give the same result as one another, and that means you need to have a way of calling each one separately.

For that reason, I recommend something like the following:

# Let's change the syntax of `replace_function`. First, fallbacks in case anyone has already made a local extension.
replace_function(f, window) = replace_function(f)
replace_function(f) = f
# Now the one you care about
function replace_function(::typeof(median!), window)
    if prod(map(length, window)) < 81
        return median_direct!
    end
    median_histogram
end

Then change all callers of replace_function(f) to replace_function(f, window), define

function median_direct!(v::AbstractVector)
    inds = indices(v,1)
    Base.middle(Base.select!(v, (first(inds)+last(inds))÷2, Base.Order.ForwardOrdering()))
end

and define median_histogram to be your definition. Then you can call this method

function _mapwindow{T,N}(f::typeof(median_histogram),

Finally, in tests you can do this:

med1 = ImageFiltering._mapwindow(ImageFiltering.median_direct!, args...)
med2 = ImageFiltering._mapwindow(ImageFiltering.median_histogram, args...)
@test med1 == med2

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have created new dispatches for different methods and separated implementations for discrete and continuous inputs. Do suggest, if there can be a better dispatch method.
Presently:

  1. Two new functions are added
    a. median_direct!
    b. median_histogram!
  2. Only for img::Union{AbstractArray{N0f8,N},AbstractArray{ColorTypes.Gray{N0f8},N}} is a choice made between median_histogram! and median_direct!.

inner = _interior(inds, window)
if callmode == :copy!
buf = Array{T}(map(length, window))
bufrs = shape(buf)
Rbuf = CartesianRange(size(buf))
offset = CartesianIndex(map(w->first(w)-1, window))
# To allocate the output, we have to evaluate f once
Rinner = CartesianRange(inner)
# Initialise the mode to zero and histogram consisting of 255 bins to zeros
mode = 0
m_histogram=zeros(Int64,(256,))
if !isempty(Rinner)
out = similar(img, typeof(f(bufrs,m_histogram,mode,window)))
Rwin = CartesianRange(map(+, window, first(Rinner).I))
copy!(buf, Rbuf, img, Rwin)
prev_mode=0
prev_col=1
for I in Rinner
curr_col=I.I[2]
if(column_change(curr_col,prev_col))
m_histogram=zeros(Int64,(256,))
prev_mode=0
end
prev_col=curr_col

Rwin = CartesianRange(map(+, window, I.I))
copy!(buf, Rbuf, img, Rwin)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm missing something, but won't this alone keep your method O(n^2) in 2d? You're copying all the values even if you only "use" some of them.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is true. Sadly, when I am trying to break Rwin in :
Rwin_add= CartesianRange(map(+, (window_array_add...), I.I)) [ Side column to add]
Rwin_subtract = CartesianRange(map(+, (window_array_subtract...), I.I)) [Side column to remove]
is leading to more running time. I can't quite figure out why that is happening. I am working on reducing the time complexity more.

# Mode 0 corresponds to refilling the empty histogram with all the points in the window
if prev_mode == 0
out[I] = f(bufrs,m_histogram,0,window)
prev_mode=1
continue
# Mode 1 corresponds to adding only the new points to the histogram and removing the old ones
elseif prev_mode == 1
out[I] = f(bufrs,m_histogram,1,window)
prev_mode=1
continue
end

end

else
copy_win!(buf, img, first(CartesianRange(inds)), border, offset)
out = similar(img, typeof(f(bufrs,m_histogram,mode,window)))
end
# Now pick up the edge points we skipped over above
for I in EdgeIterator(inds, inner)
# Handle the edge points with mode -1
mode =-1
copy_win!(buf, img, I, border, offset)
out[I] = f(bufrs,m_histogram,mode,window)
end

else
# TODO: implement :view
error("callmode $callmode not supported")
end
out
end

function column_change(curr_col,prev_col)
return (curr_col - prev_col!=0)
end

function is_medianfilter(::typeof(median!))
true
end

function is_medianfilter(::Any)
false
end

# For copying along the edge of the image
function copy_win!{T,N}(buf::AbstractArray{T,N}, img, I, border::Pad, offset)
win_inds = map(+, indices(buf), (I+offset).I)
Expand Down Expand Up @@ -143,6 +233,8 @@ function copy_win!{T,N}(buf::AbstractArray{T,N}, img, I, border::Fill, offset)
buf
end



### Optimizations for particular window-functions

mapwindow(::typeof(extrema), A::AbstractArray, window::Dims) = extrema_filter(A, window)
Expand Down Expand Up @@ -273,13 +365,100 @@ end
# This is slightly faster than a circular buffer
@inline cyclecache(b, x) = b[1], (Base.tail(b)..., x)


replace_function(f) = f
replace_function(::typeof(median!)) = function(v)
inds = indices(v,1)
Base.middle(Base.select!(v, (first(inds)+last(inds))÷2, Base.Order.ForwardOrdering()))

replace_function(::typeof(median!)) = function(v,m_histogram,mode,window)
# Handle the boundary points with mode -1
if(mode==-1)
inds = indices(v,1)
return Base.middle(Base.select!(v, (first(inds)+last(inds))÷2, Base.Order.ForwardOrdering()))
else

window_size=size(v,1)
dims = map(x->x.stop-x.start+1,window)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to use the "official interface" of ranges rather than accessing fields directly---use first(x) and last(x) rather than x.start and x.stop. (If someone renames the fields or redefines the implementation, they can't deprecate your direct access of the fields whereas they can make sure that first and last continue to work properly. This actually happened not so long ago with LinSpace and StepRangeLen.)

However, in this particular case I think this is equivalent to map(length, window), and that's much clearer for readers of your code.

width=window[1].stop-window[1].start+1
inds = indices(v,1)
if mode == 0
# Update the histogram according to new entries
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent comment too

for i = first(inds):last(inds)
id= trunc(Int64,(v[i]*255))+1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if id turns out to be < 1 or > 256? (You'll get a boundserror.) See a similar discussion here (click to expand the comments, and check the diff to see how @tejus-gupta solved the issue by calling extrema).

Would be good to add a test where the image has one value outside the 0-to-1 range.

Copy link
Author

@hari-sikchi hari-sikchi Jul 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the median_histogram method is restricted to N0f8, that won't be a problem, right?

m_histogram[id]+=1
end
counter=0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

counter = sum(m_histogram)


for i =1:256
counter+=m_histogram[i]
end
# Compute the median
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this should be a standalone function, since you do the same thing again below.

tempsum = 0
m_index=-1
for i = 1:256
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of this hard-codes the fact that histogram has length 256, but note that histogram is not even allocated in this function so people changing code in one place might cause bugs in a different place.

for in in linearindices(histogram) would be more general.

tempsum+= m_histogram[i]
if tempsum>=trunc(Int64,window_size/2)+1
m_index=i-1
break
end
end
# Clear the histogram from previous value
for i = first(inds):width:dims[1]*dims[2]
for j= i: dims[1]*dims[2]:last(inds)
println(j)
id= trunc(Int64,(v[j]*255))+1
m_histogram[id]-=1
if(m_histogram[id]<0)
println("stop")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just for debugging? How about @assert m_histogram[id] >= 0? I think julia -O3 disables assertions, so it's possible to be both safe and have it not hurt your runtime.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, pushed old commit by mistake

end
end
end
return convert(Float64,m_index)/255

elseif mode == 1
# Update the histogram according to new entries
for i = width:width:dims[1]*dims[2]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still 2d, right? How about prod(dims)?

Copy link
Author

@hari-sikchi hari-sikchi Jul 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this isn't 2D. The iterator 'i' iterates through all the new values added in all dimensions .

for i = first(inds):width:dims[1]*dims[2] [This iterates over a 2D slice]  
                for j= i: dims[1]*dims[2]:last(inds)  [This iterates over all such 2D slices in nD]

for j= i: dims[1]*dims[2]:last(inds)
id= trunc(Int64,(v[j]*255))+1
m_histogram[id]+=1
end
end
counter=0

for i =1:256
counter+=m_histogram[i]
end
println("mode:",mode)
println("counter:",counter)

# Compute the median
tempsum = 0
m_index=-1
for i = 1:256
tempsum+= m_histogram[i]
if tempsum>= trunc(Int64,window_size/2)+1
m_index=i-1
break
end
end
# Clear the histogram from previous value
for i = first(inds):width:dims[1]*dims[2]
for j= i: dims[1]*dims[2]:last(inds)
id= trunc(Int64,(v[j]*255))+1
m_histogram[id]-=1
if(m_histogram[id]<0)
println("stop")
end

end
end
return convert(Float64,m_index)/255
end

end


end

default_shape(::Any) = identity
default_shape(::typeof(median!)) = vec

end
end