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

Add Op discovery annotations to ImgLib2-algorithm #94

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build-main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
- name: Set up Java
uses: actions/setup-java@v3
with:
java-version: '8'
java-version: '11'
distribution: 'zulu'
cache: 'maven'
- name: Set up CI environment
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/build-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
- name: Set up Java
uses: actions/setup-java@v3
with:
java-version: '8'
java-version: '11'
distribution: 'zulu'
cache: 'maven'
- name: Set up CI environment
Expand Down
51 changes: 51 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,22 @@ Jean-Yves Tinevez and Michael Zinsmaier.</license.copyrightOwners>
<imglib2-roi.version>0.15.0</imglib2-roi.version>
<imglib2-cache.version>1.0.0-beta-18</imglib2-cache.version>
<sc.fiji.bigdataviewer-core.version>10.4.16</sc.fiji.bigdataviewer-core.version>

<!--
NB: Older versions of OpenJDK 11 have a bug in the javadoc tool,
which causes errors like:

[ERROR] javadoc: error - The code being documented uses packages
in the unnamed module, but the packages defined in
https://github.com/scijava/scijava/apidocs/ are in named modules.

The most recent version of OpenJDK 11 known to have this problem
is 11.0.8; the oldest version known to have fixed it is 11.0.17.
Therefore, we set the minimum build JDK version to 11.0.17 here.
-->
<scijava.jvm.build.version>[11.0.17,)</scijava.jvm.build.version>
<scijava.jvm.version>8</scijava.jvm.version>
<scijava.parse.ops>true</scijava.parse.ops>
</properties>

<repositories>
Expand Down Expand Up @@ -281,4 +297,39 @@ Jean-Yves Tinevez and Michael Zinsmaier.</license.copyrightOwners>
<scope>test</scope>
</dependency>
</dependencies>

<!-- TODO: Remove relevant sections once upstream -->
<build>
<plugins>
<plugin>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<annotationProcessorPaths>
<path>
<groupId>org.scijava</groupId>
<artifactId>scijava-ops-indexer</artifactId>
<version>1.0.0</version>
</path>
</annotationProcessorPaths>
<fork>true</fork>
<compilerArgs>
<arg>-Ascijava.ops.parse=${scijava.parse.ops}</arg>
<arg>-Ascijava.ops.opVersion=${project.version}</arg>
</compilerArgs>
</configuration>
</plugin>
<plugin>
<artifactId>maven-javadoc-plugin</artifactId>
<configuration>
<tags>
<tag>
<name>implNote</name>
<placement>a</placement>
<head>Implementation Note:</head>
</tag>
</tags>
</configuration>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public class Thresholder
* the number of threads to use for thresholding.
* @return a new {@link Img} of type {@link BitType} and of same dimension
* that the source image.
* @implNote op name='threshold.apply'
*/
public static final < T extends Type< T > & Comparable< T >> Img< BitType > threshold( final Img< T > source, final T threshold, final boolean above, final int numThreads )
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ public final class MserTree< T extends Type< T > > implements ComponentForest< M
* {@link #buildMserTree(RandomAccessibleInterval, RealType, long, long, double, double, ImgFactory, boolean)}
* using an {@link ArrayImgFactory} or {@link CellImgFactory} depending on
* input image size.
*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opinions on whether these should be Ops? I'm not totally sure what they're used for...

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

And I guess this article is relevant.

Copy link
Member

Choose a reason for hiding this comment

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

While we are figuring this stuff out, we should improve the javadoc. Every algorithm should cite the paper(s) it is about in its class javadoc. A "component tree" in the context of the componenttree packages is explained in papers like this one; without that context, it is difficult to understand what all this code is for.

Copy link
Member

Choose a reason for hiding this comment

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

Concepts of level sets, mathematical morphology, and connected component analysis are very related. We have CCA currently in labeling, but other morphology operators in morphology. What about level sets? FWIW, I checked scikit-image for comparison, and use of level sets mostly takes place in skimage.segmentation.morphsnakes.

*
* @implNote op name='create.mserTree',type=Function
* @param input
* the input image.
* @param delta
Expand Down Expand Up @@ -133,7 +134,8 @@ public static < T extends RealType< T > > MserTree< T > buildMserTree( final Ran
* {@link #buildMserTree(RandomAccessibleInterval, RealType, long, long, double, double, ImgFactory, boolean)}
* using an {@link ArrayImgFactory} or {@link CellImgFactory} depending on
* input image size.
*
*
* @implNote op name='create.mserTree',type=Function
* @param input
* the input image.
* @param delta
Expand All @@ -159,7 +161,8 @@ public static < T extends RealType< T > > MserTree< T > buildMserTree( final Ran

/**
* Build a MSER tree from an input image.
*
*
* @implNote op name='create.mserTree',type=Function
* @param input
* the input image.
* @param delta
Expand Down Expand Up @@ -198,7 +201,8 @@ public static < T extends RealType< T > > MserTree< T > buildMserTree( final Ran
* {@link #buildMserTree(RandomAccessibleInterval, ComputeDelta, long, long, double, double, ImgFactory, Type, Comparator)}
* using an {@link ArrayImgFactory} or {@link CellImgFactory} depending on
* input image size.
*
*
* @implNote op name='create.mserTree',type=Function
* @param input
* the input image.
* @param computeDelta
Expand Down Expand Up @@ -226,7 +230,8 @@ public static < T extends Type< T > > MserTree< T > buildMserTree( final RandomA

/**
* Build a MSER tree from an input image.
*
*
* @implNote op name='create.mserTree',type=Function
* @param input
* the input image.
* @param computeDelta
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ public final class PixelListComponentTree< T extends Type< T > > implements Comp
* using an {@link ArrayImgFactory} or {@link CellImgFactory} depending on
* input image size.
*
* @implNote op name='create.pixelListComponentTree',type=Function
* @param input
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Like above, should these be ops?

Copy link
Member

Choose a reason for hiding this comment

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

Can we use a sub-namespace of create here? create.componentTree.pixelList, create.componentTree.mser, and so on? It would match the package naming. In general, looking at the Java package naming is a good starting point for naming suggestions.

Copy link
Member

Choose a reason for hiding this comment

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

Actually... are these really "create" Ops? I think of create as merely allocating and returning an object instance, not doing any computation around an image. So this PixelListComponentTree shouldn't be a create Op at all, since it accepts an existing image as input. morphology.componentTree.pixelList maybe? I don't know this space of image algorithms well enough to feel qualified to suggest anything. We could use @tpietzsch's input here, since he wrote these classes. And/or just read the papers about these things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It actually seems to be like the tree best belongs to the features namespace, as it is my understanding that component trees themselves have distilled down a 2D function into a tree, where the position is lost. This seems more like feature extraction than morphology.

* the input image.
* @param type
Expand All @@ -92,6 +93,7 @@ public static < T extends RealType< T > > PixelListComponentTree< T > buildCompo
/**
* Build a component tree from an input image.
*
* @implNote op name='create.pixelListComponentTree',type=Function
* @param input
* the input image.
* @param type
Expand Down Expand Up @@ -120,6 +122,7 @@ public static < T extends RealType< T > > PixelListComponentTree< T > buildCompo
* using an {@link ArrayImgFactory} or {@link CellImgFactory} depending on
* input image size.
*
* @implNote op name='create.pixelListComponentTree',type=Function
* @param input
* the input image.
* @param maxValue
Expand All @@ -138,6 +141,7 @@ public static < T extends Type< T > > PixelListComponentTree< T > buildComponent
/**
* Build a component tree from an input image.
*
* @implNote op name='create.pixelListComponentTree',type=Function
* @param input
* the input image.
* @param maxValue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,23 @@ public class FastGauss
return new LineConvolution<>( new FastGaussConvolverRealType( sigma ), direction );
}

/**
* @param sigmas the standard deviations of the Gaussian blur (in each dimension)
* @param input the input data to blur
* @param output the preallocated output buffer
* @implNote op name='filter.gauss', type=Computer
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, interesting. We may want to make a post on Image.sc asking for people's suggestions on what Ops should do for naming w.r.t. approximate vs. exact algorithms like this. Is it OK for filter.gauss to be approximate by default? Should the exact and approximate versions have different names? Or can we use a hint somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • As we talked about Op naming, the fundamental idea is that numerical equivalence is not required for two Ops to share a name. So I think that both should be named filter.gauss. I don't like any differentiation between the names because then you're being explicit about implementation details, contrary to the Ops design.
  • Without additional changes, filter.gauss won't be approximate by default but would be approximate always, which is probably bad. Actually, right now filter.gauss(sigma, source, target) won't work because there's a conflict, and one will have to be prioritized. I'd lean towards prioritizing the approximate one for speed, and we should likely provide a hint that could be used to avoid this Op. There are likely additional scenarios where this would be useful, such as convolution in SciJava Ops Image.

As is, we need to prioritize one of the Ops before we merge this PR, and we may want to implement the Precision Hint upstream beforehand as well.

Copy link
Member

Choose a reason for hiding this comment

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

I like the idra of a precision hint. It is more general than only this Op. E.g. arbitrary precision arithmetic.

*/
public static void convolve( final double[] sigmas, final RandomAccessible< ? extends RealType< ? > > input, final RandomAccessibleInterval< ? extends RealType< ? > > output )
{
convolution( sigmas ).process( input, output );
}

/**
* @param sigma the standard deviation of the Gaussian blur (in all dimensions)
* @param input the input data to blur
* @param output the preallocated output buffer
* @implNote op name='filter.gauss', type=Computer
*/
public static void convolve( final double sigma, final RandomAccessible< ? extends RealType< ? > > input, final RandomAccessibleInterval< ? extends RealType< ? > > output )
{
convolution( sigma ).process( input, output );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ public class Kernel1D
* @param halfKernel
* the upper half (starting at the center pixel) of the symmetric
* convolution kernel.
* @return a {@link Kernel1D} used for one dimensional convolutions
* @implNote op name='create.kernel1DSymmetric'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should these be Ops? What is the right name?

Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to group all the kernel creation ops into create.kernel. sub-namespace? E.g. create.kernel.symmetric1D? Or maybe create.kernel1D.symmetric since these all return Kernel1D objects. Not sure...

*/
public static Kernel1D symmetric( final double... halfKernel )
{
Expand All @@ -68,6 +70,9 @@ public static Kernel1D symmetric( final double... halfKernel )
/**
* Similar to {@link #symmetric(double[])} but creates an array of
* one-dimensional convolution kernels.
* @param halfKernels the upper halves (starting at the center pixel) of the symmetric convolution kernels
* @return an array of {@link Kernel1D}s used for one dimensional convolutions
* @implNote op name='create.kernel1DSymmetric'
*/
public static Kernel1D[] symmetric( final double[][] halfKernels )
{
Expand All @@ -82,6 +87,8 @@ public static Kernel1D[] symmetric( final double[][] halfKernels )
* @param originIndex
* the index of the array element which is the origin of the
* kernel
* @return an asymmetric {@link Kernel1D} used for one dimensional convolutions
* @implNote op name='create.kernel1DAsymmetric'
*/
public static Kernel1D asymmetric( final double[] fullKernel, final int originIndex )
{
Expand All @@ -94,6 +101,9 @@ public static Kernel1D asymmetric( final double[] fullKernel, final int originIn
/**
* Creates a one-dimensional asymmetric convolution kernel, where the origin
* of the kernel is in the middle.
* @param kernel the raw data of the symmetric convolution kernel
* @return a {@link Kernel1D} used for one dimensional convolutions
* @implNote op name='create.kernel1DCentralAsymmetric'
*/
public static Kernel1D centralAsymmetric( final double... kernel )
{
Expand All @@ -103,6 +113,13 @@ public static Kernel1D centralAsymmetric( final double... kernel )
/**
* Similar to {@link #asymmetric(double[], int)} but creates an array of
* one-dimensional convolution kernels.
* @param fullKernels
* arrays containing the values of each kernel
* @param originIndices
* the indices of the array elements at the origin of each
* kernel
* @return an array of {@link Kernel1D}s used for one dimensional convolutions
* @implNote op name='create.kernel1DAsymmetric'
*/
public static Kernel1D[] asymmetric( final double[][] fullKernels, final int[] originIndices )
{
Expand All @@ -113,6 +130,9 @@ public static Kernel1D[] asymmetric( final double[][] fullKernels, final int[] o
/**
* Similar to {@link #centralAsymmetric(double...)} but creates an array of
* one-dimensional convolution kernels.
* @param kernels the upper halves (starting at the center pixel) of the symmetric convolution kernels
* @return an array of {@link Kernel1D}s used for one dimensional convolutions
* @implNote op name='create.kernel1DCentralAsymmetric'
*/
public static Kernel1D[] centralAsymmetric( final double[][] kernels )
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ public class SeparableKernelConvolution
* the required source interval.
* @param target
* target image.
* @implNote op name='filter.convolve', type=Computer
*/
public static void convolve( final Kernel1D[] kernels,
final RandomAccessible< ? extends NumericType< ? > > source,
Expand Down
76 changes: 76 additions & 0 deletions src/main/java/net/imglib2/algorithm/dog/DifferenceOfGaussian.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,44 @@
*/
public class DifferenceOfGaussian
{
/**
* Compute the difference of Gaussian for the input. Input convolved with
* Gaussian of sigmaSmaller is subtracted from input convolved with Gaussian
* of sigmaLarger (where {@code sigmaLarger > sigmaSmaller}).
* <p>
* Creates an appropriate temporary image and calls
* {@link #DoG(double[], double[], RandomAccessible, RandomAccessible, RandomAccessibleInterval, ExecutorService)}
* .
* </p>
* This method differs from
* {@link #DoG(double[], double[], RandomAccessible, RandomAccessibleInterval, ExecutorService)}
* only in that its parameter order is tailored to an Op. The output comes
* last, and the primary input (the input image) comes first.
*
* @implNote op name="filter.DoG", type=Computer
* @param input
* the input image extended to infinity (or at least covering the
* same interval as the dog result image, plus borders for
* convolution).
* @param sigmaSmaller
* stddev (in every dimension) of smaller Gaussian.
* @param sigmaLarger
* stddev (in every dimension) of larger Gaussian.
* @param service
* service providing threads for multi-threading
* @param dog
* the Difference-of-Gaussian result image.
*/
public static < I extends NumericType< I >, T extends NumericType< T > & NativeType< T > > void DoG(
final RandomAccessible< I > input,
final double[] sigmaSmaller,
final double[] sigmaLarger,
final ExecutorService service,
final RandomAccessibleInterval< T > dog)
{
DoG( sigmaSmaller, sigmaLarger, input, dog, service );
}

/**
* Compute the difference of Gaussian for the input. Input convolved with
* Gaussian of sigmaSmaller is subtracted from input convolved with Gaussian
Expand Down Expand Up @@ -97,6 +135,44 @@ public static < I extends NumericType< I >, T extends NumericType< T > & NativeT
DoG( sigmaSmaller, sigmaLarger, input, Views.translate( g1, translation ), dog, service );
}

/**
* Compute the difference of Gaussian for the input. Input convolved with
* Gaussian of sigmaSmaller is subtracted from input convolved with Gaussian
* of sigmaLarger (where sigmaLarger &gt; sigmaSmaller).
* </p>
* This method differs from
* {@link #DoG(double[], double[], RandomAccessible, RandomAccessible, RandomAccessibleInterval, ExecutorService)}
* only in that its parameter order is tailored to an Op. The output comes
* last, and the primary input (the input image) comes first.
*
* @implNote op name="filter.DoG", type=Computer
gselzer marked this conversation as resolved.
Show resolved Hide resolved
* @param input
* the input image extended to infinity (or at least covering the
* same interval as the dog result image, plus borders for
* convolution).
* @param sigmaSmaller
* stddev (in every dimension) of smaller Gaussian.
* @param sigmaLarger
* stddev (in every dimension) of larger Gaussian.
* @param tmp
* temporary image, must at least cover the same interval as the
* dog result image.
* @param service
* how many threads to use for the computation.
* @param dog
* the Difference-of-Gaussian result image.
*/
public static < I extends NumericType< I >, T extends NumericType< T > & NativeType< T > > void DoG(
gselzer marked this conversation as resolved.
Show resolved Hide resolved
final RandomAccessible< I > input,
final double[] sigmaSmaller,
final double[] sigmaLarger,
final RandomAccessible< T > tmp,
final ExecutorService service,
final RandomAccessibleInterval< T > dog)
{
DoG(sigmaSmaller, sigmaLarger, input, tmp, dog, service);
}

/**
* Compute the difference of Gaussian for the input. Input convolved with
* Gaussian of sigmaSmaller is subtracted from input convolved with Gaussian
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ public class SubpixelEdgelDetection
* <p>
* Note: The input image type must be a signed type! Otherwise gradient
* computation will not work.
*
*
* @implNote op name='image.subpixelEdgels', type=Function
gselzer marked this conversation as resolved.
Show resolved Hide resolved
* @param input
* input image
* @param factory
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/net/imglib2/algorithm/fill/FloodFill.java
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ public class FloodFill
* input pixel type
* @param <U>
* fill label type
* @implNote op name='image.floodFill', type=Computer2
gselzer marked this conversation as resolved.
Show resolved Hide resolved
*/
public static < T extends Type< T >, U extends Type< U > > void fill(
final RandomAccessible< T > source,
Expand Down Expand Up @@ -138,6 +139,7 @@ public static < T extends Type< T >, U extends Type< U > > void fill(
* input pixel type
* @param <U>
* fill label type
* @implNote op name='image.floodFill', type=Computer2
*/
public static < T, U extends Type< U > > void fill(
final RandomAccessible< T > source,
Expand Down Expand Up @@ -180,6 +182,7 @@ public static < T, U extends Type< U > > void fill(
* input pixel type
* @param <U>
* fill label type
* @implNote op name='image.floodFill', type=Computer2
*/
public static < T, U > void fill(
final RandomAccessible< T > source,
Expand Down
Loading
Loading