Skip to content

Conversation

@maarzt
Copy link
Contributor

@maarzt maarzt commented Oct 9, 2017

Separable asymmetric kernels are used in image processing.
One example: The derivative of a gauss kernel is a separable, asymmetric kernel, and is used to calculate exact derivatives of an gaussian blurred image.

This PR deprecates the current separable symmetric convolution algorithm. And adds generalized algorithm for separable convolution. The new algorithm has been benchmarked against the current implementation an proved to be just as fast.

Gauss3 is changed to use the new implementation.

@maarzt
Copy link
Contributor Author

maarzt commented Oct 9, 2017

Do we want to move the classes for separable (symmetric) convolution to net.imglib2.algorithm.convolution? (done)

@maarzt maarzt force-pushed the seperable-convolution branch 3 times, most recently from f190097 to a2b3399 Compare November 9, 2017 09:44
@maarzt
Copy link
Contributor Author

maarzt commented May 1, 2018

I think this PR is ready to be merged and I need it to implement features for pixel classification.

@maarzt maarzt requested a review from tpietzsch May 1, 2018 01:03
@maarzt maarzt force-pushed the seperable-convolution branch from a2b3399 to c2718c2 Compare May 1, 2018 01:12
@maarzt maarzt changed the title Seperable Convolution with Asymmetric Kernels Seperable Convolution with Asymmetric Kernels (depends on https://github.com/imglib/imglib2/pull/206) May 2, 2018
@maarzt maarzt changed the title Seperable Convolution with Asymmetric Kernels (depends on https://github.com/imglib/imglib2/pull/206) Seperable Convolution with Asymmetric Kernels (imglib2 PR #206) May 2, 2018
@maarzt maarzt force-pushed the seperable-convolution branch from 43b47b5 to 574e43e Compare July 30, 2018 13:36
@maarzt maarzt changed the title Seperable Convolution with Asymmetric Kernels (imglib2 PR #206) Seperable Convolution with Asymmetric Kernels + Fast Gauss Jul 30, 2018
* <p>
* The result is a good approximation to "real" gauss. But the
* same holds for {@link net.imglib2.algorithm.gauss3.Gauss3}.
* Which one is better is not known so far.
Copy link
Member

Choose a reason for hiding this comment

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

This comment is not very helpful IMO. Would be better to state what the difference in approximation is.
I.e. Gauss3 does a (non-approximated) convolution with a Gaussian kernel truncated at 3*sigma+1.
What is the approximation here?

Also, you should add a reference to the paper that is implemented here.

final double[][] halfkernels = halfkernels( sigma );
final ExecutorService service = Executors.newFixedThreadPool( numThreads );
SeparableSymmetricConvolution.convolve( halfkernels, source, target, service );
SeparableKernelConvolution.convolve( Kernel1D.symmetric( halfkernels ), source, target );
Copy link
Member

Choose a reason for hiding this comment

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

This does not use the provided ExecutorService

{
final double[][] halfkernels = halfkernels( sigma );
SeparableSymmetricConvolution.convolve( halfkernels, source, target, service );
SeparableKernelConvolution.convolve( Kernel1D.symmetric( halfkernels ), source, target );
Copy link
Member

Choose a reason for hiding this comment

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

This does not use the provided ExecutorService

/**
* Return an object, that performs the separable convolution with the give kernel.
* It additional allows to set the {@link java.util.concurrent.ExecutorService} to use
* or to query for the required input image size, ...
Copy link
Member

Choose a reason for hiding this comment

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

Typos: "give" --> "given"
"additional" --> "additionally"

maarzt added 5 commits August 20, 2018 15:46
It's faster than the older one and allows asymmetric kernels.
It's functionality is replaced by SeparableKernelConvolution.
It is an approximate algorithm to calculate Gaussian blur.
But way faster for sigma greater than 3.
@maarzt maarzt force-pushed the seperable-convolution branch from 574e43e to 64429bb Compare August 20, 2018 13:49
@tpietzsch
Copy link
Member

@maarzt Could you please fix, what I have commented on so far?

@maarzt
Copy link
Contributor Author

maarzt commented Aug 29, 2018

@tpietzsch I made all the changes you required. And finally I cited the paper describing the fast gauss algorithm that is implemented.

@tpietzsch
Copy link
Member

@maarzt Did you push your changes? Gauss3 still doesn't use user-provided ExecutorService

maarzt and others added 16 commits August 31, 2018 15:47
  * Use fork( 1 ) to avoid potential dependency of results on execution order.
  * Longer warmup for GaussBenchmark.
This makes it clear that benchmarkSeparableKernelConvolution() is using the
same number of threads as benchmarkSeparableSymmetricConvolution().
The tests are no longer needed, because they test the same functionality
as ConvolverTest.
…e except DoubleType

This is required for higher precision when blurring for example
ByteType images.
@maarzt
Copy link
Contributor Author

maarzt commented Oct 17, 2018

@tpietzsch I did the changes you asked for:

  1. Intermediate results for convolution of all RealTypes except DoubleType is now FloatType.
  2. I use assertImageEquals from new imglib2-5.6.0

@tpietzsch tpietzsch merged commit de30b1a into imglib:master Oct 18, 2018
@tpietzsch
Copy link
Member

Great, thanks!

@maarzt maarzt deleted the seperable-convolution branch August 27, 2019 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants