downsample the image before clustering #71
Conversation
Codecov Report
@@ Coverage Diff @@
## master #71 +/- ##
==========================================
+ Coverage 98.44% 98.49% +0.04%
==========================================
Files 14 14
Lines 257 265 +8
==========================================
+ Hits 253 261 +8
Misses 4 4
Continue to review full report at Codecov.
|
johnnychen94
changed the title
This commit also adds ImageBase dependency; a drop-in replacement for ImageCore with a few handy operators.
src/clustering.jl
Outdated
| # Clustering on the downsampled image already generates good enough colormap estimation | ||
| # This significantly reduces the algorithmic complexity. | ||
| img = _restrict_to(img, ncolors*100) | ||
| data = convert(Array{Float32}, reshape(channelview(Lab.(img)), 3, :)) |
There was a problem hiding this comment.
Is there a reason for the added conversion?
There was a problem hiding this comment.
For many big-data problems, it's often a good strategy to eagerly collect data into a contiguous memory layout so that future slicing on data[inds...] can be faster.
I added this before trying the downsample trick only to verify if this helps reduce the allocations. Here I don't observe any performance difference, I guess it's because Clustering.jl is doing a similar thing on their side.
I've reverted this in the new commit.
For any real-world image, clustering the image pixel is a computational intensive big-data problem. For our very purpose to estimate the colormap, a downsampled
the version already works well.
The results are not identical but the visual quality looks quite similar. I think this is a good improvement overall (not a breaking change).