feature_transform: add multithreading
#47
Conversation
timholy
force-pushed
the
teh/parallelize_feat
branch
2 times, most recently
from
18ed6fb to
7bae26f
Compare
There was a problem hiding this comment.
A very nice work!
I am unsure about the generalization to Gray{Bool}; Julia doesn't treat anything beside Bool as a bool in if statements, but OTOH when we load a "binary image" it comes in as Gray{Bool}. I'd be interested in the thoughts of others.
I wouldn't be surprised if people try to directly pass the output from ImageBinarization.binarize (Gray{Bool}) into feature_transform.
| function feature_transform(I::AbstractArray{Bool,N}, w::Union{Nothing,NTuple{N}}=nothing) where N | ||
| function feature_transform(img::AbstractArray{<:Union{Bool,AbstractGray{Bool}},N}; | ||
| weights::Union{Nothing,NTuple{N}}=nothing, | ||
| nthreads::Int = length(img) < 1000 ? 1 : Threads.nthreads()) where N |
There was a problem hiding this comment.
Curious to ask, does this mean that ComputationalResources.jl is considered deprecated?
There was a problem hiding this comment.
I'm glad you ask. I'm not sure how to think about that. It was added before keyword arguments to give meaning to options that otherwise might be too simple to be unambiguous (as you've noted, imfilter has a complicated dispatch hierarchy and could be cleaned up with keyword arguments). That said, it does provide facilities for passing options, and that's not a bad thing. I could incorporate it here. Just not quite sure what to think...
There was a problem hiding this comment.
From a more general perspective, I could imagine that algorithm f is expected to accept img::CuArray and calls the CUDA implementation, in which there won't be nthreads keyword.
The difficulty in ComputationalResources is that if we don't separate the interface dispatch from the implementation dispatch, we need to handle the complicated dispatch hierarchy for every function. I personally think it is affordable to maintain one or two complicated dispatch hierarchies using ComputationalResources. So for example, in ImageBinarization we can only configure CR for binarize/binarize!, and leave all the implementation dispatch to concrete algorithms, e.g., https://github.com/zygmuntszpak/ImageBinarization.jl/blob/0c0c5eaefa997f07746c24887c9ed5fad6199d9b/src/algorithms/niblack.jl#L84-L85
Said this, the current nthreads keyword version looks good to me right now. We can reraise this discussion when we have more concrete issues.
timholy
force-pushed
the
teh/parallelize_feat
branch
2 times, most recently
from
d3b8e06 to
5efaffd
Compare
If Julia is being run with multiple threads, this now will default to using all threads in `feature_transform`. The multithreaded implementation is relatively straightforward, although the recursive algorithm makes it a little hard to wrap your head around: - `computeft!` for dimension `d` is dependent only on slices of dimensions `1:d`, and indeed for all except the final call to `voronoift!` just on dimensions `1:d-1`. Hence you can divide the last dimension `N` into chunks, skip the final `voronoift!` on dimension `N`, and give each to a thread. - For the final `voronoift!` call along dimension `N`, it's a one-dimensional operation along this dimension. Hence you can just split the next-to-last dimension into chunks instead. This change was responsible for performance improvements in `distance_transform` noted in JuliaImages/image_benchmarks#1. Julia 1.3 is required for `@spawn` Co-authored-by: Johnny Chen <[email protected]>
timholy
force-pushed
the
teh/parallelize_feat
branch
from
5efaffd to
41f9fa8
Compare
`feature_transform`: add multithreading
If Julia is being run with multiple threads, this now will default
to using all threads in
feature_transform. The multithreadedimplementation is relatively straightforward, although the recursive algorithm
makes it a little hard to wrap your head around:
computeft!for dimensiondis dependent only on slices of dimensions1:d,and indeed for all except the final call to
voronoift!just on dimensions1:d-1.Hence you can divide the last dimension
Ninto chunks and give each to a thread.For the final
voronoift!call along dimensionN, it's aone-dimensional operation along this dimension. Hence you can just
split the next-to-last dimension into chunks instead.
This change was responsible for performance improvements in
distance_transformnoted in JuliaImages/image_benchmarks#1.
I am unsure about the generalization to
Gray{Bool}; Julia doesn't treat anything besideBoolas a bool inifstatements, but OTOH when we load a "binary image" it comes in asGray{Bool}. I'd be interested in the thoughts of others.