ImageFiltering.jl icon indicating copy to clipboard operation
ImageFiltering.jl copied to clipboard

TODOs

Open timholy opened this issue 4 years ago • 7 comments

There seem to be a number of people who are interested in making improvements in this package. To facilitate their explorations, I thought I'd list some things I know need fixing:

  • [x] Write more benchmarks (~~see #152~~#158)
  • [ ] Reorganize the documentation (see #153)
  • [ ] (PR in #154) Update usage of OffsetArrays: this package was developed before OffsetArrays was particularly efficient, and it contains ugly workarounds to strip off the OffsetArray wrapper before performing the operation. Get rid of these workarounds, and document/fix any resulting performance changes. (There might be performance improvements as well as a risk of regressions.) ~~Be aware of https://github.com/JuliaArrays/OffsetArrays.jl/pull/90 which should arrive fairly soon.~~
  • [ ] Fix inefficiencies of TiledIteration. That package is potentially a huge performance booster, but https://stackoverflow.com/questions/47590839/unexpected-memory-allocation-when-using-array-views-julia/47607539#47607539 gets in the way. If the wrapper creation and usage can be inlined into a single caller, then the compiler will elide the wrapper creation. Also be aware of false sharing and the potential performance problems it can cause (good thread: https://discourse.julialang.org/t/parallel-reductions/30180)
  • [ ] Look into whether the boundary conditions can be implemented more generally as views, perhaps moving more functionality to PaddedViews.
  • [ ] Check whether the threading implementation should be updated for Julia 1.3's new threading model
  • [ ] Implement threading for non-separable FIR filtering. Currently it's implemented only for separable kernels, because that's what I use most.
  • [ ] GPUs...
  • [ ] Consider splitting the package into ArrayFiltering, keeping anything color-related in ImageFiltering (which would become a thin wrapper around ArrayFiltering). Lots of people who don't do image processing are turned off by the "Image" in the name; it's not rational, but it's been brought up again and again, so it's time to stop wishing others would change and just change ourselves.

timholy avatar Jan 13 '20 15:01 timholy

~The last one seems really difficult and could lead to a lot of fruitless effort. For it to be successful I think we'd need active participation from the wider non JuliaImages community.~ Nevermind, I didn't think about who I was talking to. JuliaArrays sounds like the perfect place for this.

Tokazama avatar Jan 13 '20 16:01 Tokazama

It's probably not that hard. Aside from function renaming (imfilter->??), it's probably just a question of splitting anything that is T<:Colorant out from everything else. ImageFiltering might become less than 50 lines of code, while @reexporting ArrayFiltering. I also have authority to transfer to JuliaArrays, assuming that's a more natural place for it.

timholy avatar Jan 13 '20 18:01 timholy

There might be low-hanging fruit in using LoopVectorization.jl to speed up image filtering. Or it might hang high, but I know there's fruit out there.

GunnarFarneback avatar Jan 13 '20 19:01 GunnarFarneback

Yes, I have my eye on LoopVectorization, it's an awesome package. Pretty brittle at the moment, but I expect that to change (and perhaps whoever digs into this can help it mature).

timholy avatar Jan 13 '20 22:01 timholy

* Look into whether the boundary conditions can be implemented more generally as views, perhaps moving more functionality to [PaddedViews](https://github.com/JuliaArrays/PaddedViews.jl).

A quick clarification, is the idea to replace BorderArray?

https://github.com/JuliaImages/ImageFiltering.jl/blob/11726578463e0f0190eab7aabe96d6a6d8c7d948/src/borderarray.jl#L57-L69

goretkin avatar Sep 10 '20 20:09 goretkin

Yes, the idea is to break ImageFiltering into multiple generic standalone small packages(for bordering, scheduling, mapwindow, filter, etc), and evolve it to a glue package.

People are sometimes confused about the image prefix because it works out pretty well for common arrays, see also https://github.com/JuliaImages/ImageFiltering.jl/issues/42 for some discussion about this.

johnnychen94 avatar Sep 10 '20 20:09 johnnychen94

Though I would point out that to split into multiple packages we don't necessarily need to split it into multiple repositories. I am using a single repository for SnoopCompileCore & SnoopCompile (two packages that can be separately installed but really designed to work together), and after a few hiccups that's working reasonably well.

timholy avatar Sep 10 '20 22:09 timholy