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

Hough circle transforms. #526

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Hough circle transforms. #526

wants to merge 1 commit into from

Conversation

tinevez
Copy link
Member

@tinevez tinevez commented Oct 19, 2017

Package for Hough circle transform in ops, based on Bresenham circle iteration.

To be merged after discussion with the work of @gselzer on https://github.com/imagej/imagej-ops/blob/54ba352f1f28ccf912806d8781529c1fb15f03a0/src/main/java/net/imagej/ops/segment/hough/CircleTransform.java

Example (requires imglib2-ij and imagej-ops...)

ImageJ.main( args );

final Context context = new Context();
final OpService ops = context.getService( OpService.class );


/*
 * Create two simple circle.
 */

final int w = 256;
final Img< BitType > img = ops.create().img( FinalDimensions.wrap( new long[] { w, w } ), new BitType() );

final int r1 = 50;
final Point c1 = new Point( w / 2 - r1, w / 2 );
MidPointAlgorithm.set( img, c1, r1, new BitType( true ) );

final int r2 = 75;
final Point c2 = new Point( w / 2 + r2 / 2, w / 2 );
MidPointAlgorithm.set( img, c2, r2, new BitType( true ) );

/*
 * Hough transform.
 */

final long minCircleRadius = 30;
final long maxCircleRadius = 100;
final long stepRadius = 3;
@SuppressWarnings( { "unchecked", "rawtypes" } )
final HoughTransformOpNoWeights< BitType > hough = ( HoughTransformOpNoWeights ) Hybrids.unaryCF( ops,
		HoughTransformOpNoWeights.class,
		Img.class, img,
		minCircleRadius, maxCircleRadius, stepRadius );

final Img< DoubleType > voteImg = hough.calculate( img );

final ImagePlus imp = ImageJFunctions.show( img, "Two circles" );
ImageJFunctions.show( voteImg, "Vote" );

/*
 * Hough detection.
 */
	
final double sensitivity = 5.;
@SuppressWarnings( { "rawtypes", "unchecked" } )
final HoughCircleDetectorLocalExtremaOp< DoubleType > houghDetector = ( HoughCircleDetectorLocalExtremaOp ) Functions.unary( ops,
		HoughCircleDetectorLocalExtremaOp.class, 
		List.class, voteImg, 
		minCircleRadius, stepRadius, sensitivity );
final List< HoughCircle > circles = houghDetector.calculate( voteImg );
System.out.println( "Found " + circles.size() + " circles:" );
imp.setOverlay( new Overlay() );
for ( final HoughCircle circle : circles )
{
	final OvalRoi roi = new OvalRoi(
			circle.getDoublePosition( 0 ) - circle.getRadius(),
			circle.getDoublePosition( 1 ) - circle.getRadius(),
			2. * circle.getRadius(),
			2. * circle.getRadius() );
	imp.getOverlay().add( roi );
	System.out.println( circle );
}
imp.updateAndDraw();

Output:

Found 2 circles:
(165.0,128.0)	R=75.0	Sensitivity=1.1
(77.3,127.3)	R=51.1	Sensitivity=3.4

two circles-1

vote-1

@ctrueden
Copy link
Member

Thanks @tinevez. Somehow, you and @gselzer missed each others' comments earlier I think. But @gselzer is definitely now tuned in to this work. He'll go over this PR, and figure out the best way to proceed. We'll respond back with any questions, etc.

@tinevez
Copy link
Member Author

tinevez commented Oct 20, 2017

Yes my bad.
I actually need help on this PR to get the ops namespace and matching right. As of now matching does not work as it should I think.
Plus I would like to try and propose a multithreaded version of the transform itself.

@gselzer
Copy link
Contributor

gselzer commented Oct 21, 2017

Hey @tinevez!
I am really sorry I did not see your messages; my notifications were going to the one email that I do not receive notifications for (I changed it now, so I should see the notifications to this thread).

I looked through your implementation, and overall, though there are parts that I still do not understand, it looks REALLY good. I have yet to test your code out for myself, but I did have a few questions that came up in my coding that I wanted to ask you about.

  • I would suggest that the two transform ops themselves extend RealType instead of BooleanType, because this provides us with a lot more flexibility. RealType input would allow the user to input not only binary masks (supposing they use BitType and not BooleanType, which is probably safe since there are no BooleanType convert Ops), but this would allow them to also input any image that has gone through edge detection algorithms either currently or in future ImageJ that output RealType masks. I actually found it really hard to take an image and make it into a BinaryType for testing in my implementation, and maybe users might have the same difficulty.
  • I would also suggest that we use @awalter17 's new ROIs for this implementation, as this could future-proof the HoughCircle (this HoughCircle could even extend Sphere.) This might be helpful as then we could use all of the functions of the new ROIs with the results of the transform. You can download the snapshot dependency from here at the shape-rois branch.
    These were the big things. I will check more on Tuesday, since it is such a nice day here in Wisconsin and I wanted to just convey the big things I saw so far ASAP. I can also help with the namespace on Tuesday.

If you would like to look at what I did, you can find it here. It is very much still a WIP, but it has been giving very nice results save for the variable-radius partial-circles with variable-thickness edges image that I have been testing it on recently (shameless plug). Let me know what you think, but I personally like your implementation a little better.

Best,
Gabe

@tinevez
Copy link
Member Author

tinevez commented Oct 23, 2017

Hi @gselzer

Yes sorry, I was incubating this for a related project (https://github.com/tinevez/CircleSkinner) and then realised you were on Hough circle too while preparing this PR. Anyway we can make the best out of our 2 PRs I am sure.

I looked through your implementation, and overall, though there are parts that I still do not understand,

Can you point me to them? Let's do a review by you on my code, I am sure there are stuff that you could improve by pointing me at the non-clear parts.

I would suggest that the two transform ops themselves extend RealType instead of BooleanType, because this provides us with a lot more flexibility. RealType input would allow the user to input not only binary masks (supposing they use BitType and not BooleanType, which is probably safe since there are no BooleanType convert Ops), but this would allow them to also input any image that has gone through edge detection algorithms either currently or in future ImageJ that output RealType masks. I actually found it really hard to take an image and make it into a BinaryType for testing in my implementation, and maybe users might have the same difficulty.

I am not too sure about this.
The input of the Hough transform ops are binary images: You create a series of circles in the vote image for every 'on' pixel in the binary input. The Hough transform itself does not know how to deal with non-binary images.

Normally you would threshold the edge image (defined over RealType) to generate a binary image from it, but this has to be a separate operation, lest we do not achieve proper separation of concerns.

So I would suggest to really limit the Hough transform ops as ops that takes a binary image and output a vote image. Coming up with the binary image is another process that can be achieved in a variety of ways.

I would also suggest that we use @awalter17 's new ROIs for this implementation, as this could future-proof the HoughCircle (this HoughCircle could even extend Sphere.) This might be helpful as then we could use all of the functions of the new ROIs with the results of the transform. You can download the snapshot dependency from here at the shape-rois branch.

Yes this one the weak point here. The output of the detection process is a custom class called HoughCircle. Could we keep it like this until the ROI project is merged in the master?

@ctrueden
Copy link
Member

ctrueden commented Oct 23, 2017

@gselzer wrote:

I actually found it really hard to take an image and make it into a BinaryType for testing in my implementation

Here is a Groovy script which binarizes an image using the ImgLib2 Converters utility class:

#@ Img img

import net.imglib2.RandomAccessibleInterval
import net.imglib2.type.logic.BitType
import net.imglib2.converter.Converters

Converters.convert(img as RandomAccessibleInterval,
	{ i, o -> o.set(i.get() > 150) }, new BitType())

It should be straightforward to do something similar in Java.

@gselzer
Copy link
Contributor

gselzer commented Oct 24, 2017

@tinevez wrote:

Normally you would threshold the edge image (defined over RealType) to generate a binary image from it, but this has to be a separate operation, lest we do not achieve proper separation of concerns.

I suppose you are right about separation of concerns. I still think that the convenience that the op would give by using RealType is still there, but considering there is a 5 line script (thanks @ctrueden) to solve the obstacle that I saw from the novice user's perspective while still separating the operations needed, I no longer see the problem with using BinaryType.

The output of the detection process is a custom class called HoughCircle. Could we keep it like this until the ROI project is merged in the master?

I don't have any strong feelings one way or the other. We are going to have to either refactor the whole of HoughCircle or, if we refactor now, will have to remove the snapshot dependency after the project is merged. I think it comes down to how immediately it is needed, and, if the answer is sooner than the merge of ROIs, then I think leave it. We just have to remember to refactor once that merge comes around.

@gselzer
Copy link
Contributor

gselzer commented Oct 24, 2017

I had the opportunity to test your implementation against possibly the meanest image that I could think of. I drew 25 circles, some of them more visible than others:
mean_image

Here is the result from LocalExtrema, with no params changed :

mean_image_local_extrema

Found 12 circles:
(153.9,655.1)	R=79.0	Sensitivity=1.5
(204.0,111.0)	R=49.0	Sensitivity=1.5
(1174.6,238.4)	R=88.0	Sensitivity=1.7
(277.6,83.4)	R=64.0	Sensitivity=1.8
(666.5,280.3)	R=148.1	Sensitivity=2.4
(998.7,611.6)	R=67.0	Sensitivity=2.5
(761.2,497.3)	R=214.3	Sensitivity=2.5
(957.3,721.7)	R=67.0	Sensitivity=2.5
(1433.3,75.7)	R=76.0	Sensitivity=2.6
(219.0,270.0)	R=85.1	Sensitivity=3.0
(365.4,585.0)	R=108.5	Sensitivity=3.0
(378.8,175.1)	R=43.0	Sensitivity=4.4

The implementation seems to have the largest problem with circles inside of circles. Is there a way we can fix this?

For some reason, I could not get the DOG circle finder to work. I tried sigma values of 0.5, 1, and 2, and the result was 0 circles every time.

		@SuppressWarnings( { "rawtypes", "unchecked" } )
		final HoughCircleDetectorDogOp< DoubleType > houghDetector = ( HoughCircleDetectorDogOp ) Functions.unary( ops,
				HoughCircleDetectorDogOp.class, 
				List.class, voteImg, 
				minCircleRadius, stepRadius, sigma, sensitivity );

@ctrueden is also suggesting that we fill this image with noise, to see how the filter deals with that, but I am afraid that I will not have time to check this until Thursday! I will also do the review then.

Best,

Gabe

@tinevez
Copy link
Member Author

tinevez commented Oct 25, 2017

Hi @gselzer
Hey thanks!

The implementation seems to have the largest problem with circles inside of circles. Is there a way we can fix this?

This is more or less by construction. If you used the local maxima circle detector, it does non-maxima suppression, which ends up in removing included circle.

For the DoG detection to work, you need to increase the sensitivity 5x or 10x compared to what you use for local maxima detection.

Could you post the code you used for the test?

Copy link
Contributor

@gselzer gselzer left a comment

Choose a reason for hiding this comment

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

I read through the implementation, and it looks really good. There were a few minor things and a few things that I was unsure of, but overall it was really well designed. I think that the only thing that absolutely needs changing is the min/max radius stuff though.

// Get a suitable image factory.
final int numDimensions = input.numDimensions();
if ( input.numDimensions() != 2 ) { throw new IllegalArgumentException(
"Cannot compute Hough circle transform for non-2D images. Got " + numDimensions + "D image." ); }
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be needed because of your Contingent below.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok.

Copy link
Contributor

@gselzer gselzer Oct 31, 2017

Choose a reason for hiding this comment

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

So I think that the best solution to this problem is to actually keep the Contingent interface and remove the runtime check. @ctrueden and I discussed a few reasons for this.

  • The Contingent interface is used to prevent Op Matching based on factors other than input types, e.g. Number of Dimensions, like you had before. If we keep the Contingent then it will simply overlook the op if we input a non-2D image, instead of attempting the op and throwing an error.
  • Let's say that, in the future, we want to make a Hough Circle/Sphere Transform that takes in only 3 Dimensions, or something else along these lines that does not involve the op signature itself. We would have to refactor this op instead of make a new one, which goes against the extensible case-logic that Ops should be.
  • The run-time checking could potentially be time consuming. Let's say that instead of using ops().run() the user calls ops().op(), which would return an Op reference that the user could use to run through a bunch of images, instead of calling ops().run() with every image. While the user could theoretically pass an image into this reference that would not be caught by conforms, it is part of the social contract with the user that they will not do this. And if there are a lot of images run over this reference, then it could hamper run time if the reference is used many times.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok!

{
final int numDimensions = input.numDimensions();
if ( input.numDimensions() != 2 ) { throw new IllegalArgumentException(
"Cannot compute Hough circle transform for non-2D images. Got " + numDimensions + "D image." ); }
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, Contingent usage

"Cannot compute Hough circle transform for non-2D images. Got " + numDimensions + "D image." ); }

maxRadius = Math.max( minRadius, maxRadius );
minRadius = Math.min( minRadius, maxRadius );
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, temp variable required or else overwrite of smaller value is possible.

"Cannot compute Hough circle transform for non-2D images. Got " + numDimensions + "D image." ); }

maxRadius = Math.max( minRadius, maxRadius );
minRadius = Math.min( minRadius, maxRadius );
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, temp variable required

public void compute( final IterableInterval< T > input, final Img< DoubleType > output )
{
final int numDimensions = input.numDimensions();
if ( input.numDimensions() != 2 ) { throw new IllegalArgumentException(
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, Contingent usage

final List< Circle > retained = new ArrayList<>();
NEXT_CIRCLE: for ( final Circle tested : peaks )
{
for ( final Circle kept : retained )
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain to me the purpose of removing any circles inside circles? I do not understand this.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the non-maxima suppression part.
The local extrema-based detector directly looks for local maxima in the vote image.

The vote image might be 'noisy': several local maxima might correspond to the same true circle but be separated by a couple of pixels. This will results in having two or more circles detected very close to each other with a very similar radius.

Non-maxima suppression is an ad-hoc workaround for these artefacts. You order all the circles by some quality factor (we use the sensitivity) and if two circles are found too close (in my case, one inside one another) you just keep the one with the best quality (lower sensitivity).

This is not always desirable if you know that some smaller circles are supposed to be inside larger one. In that case you have to use the other detector.

Copy link
Contributor

Choose a reason for hiding this comment

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

So if we use the DOG Detector then we will find smaller circles inside larger ones?

{
private final double radius;

private final double sensitivity;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also explain to me what sensitivity is measuring?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hough circle detection is a detection algorithm. Many of them returns a quality measure for what they detect (spot, circle, line,...) that reports how likely it is that it is not a spurious detection. The greater the quality, the more likely that it is a real one.

In this case I chose to use sensitivity which varies as the inverse of this quality. The sensitivity of a circle appears as the minimal value the sensitivity settings must be set to incorporate it in the results.

In the UI, the user would change a sensitivity settings, and all the circles with a sensitivity higher than this threshold would be discarded.

Copy link
Contributor

Choose a reason for hiding this comment

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

Aha! Now I understand.

if ( input.numDimensions() != 2 ) { throw new IllegalArgumentException(
"Cannot compute Hough circle transform for non-2D images. Got " + numDimensions + "D image." ); }

maxRadius = Math.max( minRadius, maxRadius );
Copy link
Contributor

Choose a reason for hiding this comment

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

If the user inputs the smaller value in maxRadius and the larger value in minRadius then the smaller value will be overwritten with this logic. You need a temporary value here to store the value of maxRadius in the case that it is overwritten, I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

Doh! My bad.

}

@Override
public void fwd()
Copy link
Contributor

Choose a reason for hiding this comment

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

I am no expert on time, but it seems to me like it would be much easier and maybe even faster to, at every radius, do this just once to create a lookup table, and then just access the lookup table every time this is called. Would that not make sense here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Normally this is how circles are painted on the screen (https://en.wikipedia.org/wiki/Midpoint_circle_algorithm). It is supposed to be very efficient and can deal with whatever radius we pick, without too much memory allocation.

@tinevez
Copy link
Member Author

tinevez commented Oct 30, 2017

Hi @gselzer
I tried to address all the points you raised.
Can you tell me whether it's ok?

I chose not to rely on the Contingent interface in the end, as it seems that it is more like something we use to refine the op matching step.

@gselzer
Copy link
Contributor

gselzer commented Oct 31, 2017

Hey @tinevez!

So I think with Contingent it is better to leave it in and to remove the runtime check. I wrote a pretty detailed post above after discussing it with @ctrueden.

I performed the DOG Detector again on my test image, and I used sensitivity values of 25., 50., and got nothing. I realized that I was probably going the wrong way then, because you described how sensitivity is an inverse, and tried 1. and got this image. I got a similar looking image for 0.5, and the only parameter changed from the test above was the sensitivity (i.e. min- and max radius were not changed):

mean_image_dog_sequals1

Found 17 circles:
(761.5,497.2)	R=61.3	Sensitivity=6.8
(666.2,279.3)	R=39.3	Sensitivity=9.2
(153.9,655.2)	R=16.0	Sensitivity=13.3
(1030.1,297.9)	R=54.9	Sensitivity=15.0
(755.0,498.7)	R=62.8	Sensitivity=16.3
(1031.2,296.9)	R=53.2	Sensitivity=16.8
(1028.0,298.1)	R=53.1	Sensitivity=17.8
(363.9,586.4)	R=25.7	Sensitivity=18.1
(365.6,584.9)	R=25.7	Sensitivity=18.1
(365.6,588.1)	R=25.7	Sensitivity=18.1
(1029.1,293.9)	R=54.0	Sensitivity=18.3
(1030.9,299.6)	R=53.2	Sensitivity=18.3
(1034.2,298.3)	R=54.0	Sensitivity=18.3
(752.1,497.2)	R=63.7	Sensitivity=18.4
(1180.0,535.4)	R=49.0	Sensitivity=19.5
(1188.3,536.0)	R=49.0	Sensitivity=19.5
(1189.0,527.6)	R=49.0	Sensitivity=19.6

The DOG Op seems to have missed a large portion of the medium/small circles, even more than the LocalExtrema missed, and, based on the data above, it looks like there are a lot of repeats. Do you know what could be causing this?

@tinevez
Copy link
Member Author

tinevez commented Oct 31, 2017

Hi @gselzer
No I do not know. Can you post the test code you used?

@gselzer
Copy link
Contributor

gselzer commented Nov 2, 2017

@tinevez Here is the code I used.

		Context c = new Context();
		
		final ImageJ ij = new ImageJ(c);
		
		ij.launch( args );

		final Context context = new Context();
		final OpService ops = context.getService( OpService.class );
		
	  Img<UnsignedByteType> input = (Img<UnsignedByteType>) ij.io().open("/home/gabe/Desktop/Mean_Image.tif");
	  Img<BitType> img = (Img<BitType>) ij.op().run(Ops.Convert.Bit.class, input);

		/*
		 * Hough transform.
		 */
		final long minCircleRadius = 40;
		final long maxCircleRadius = 250;
		final long sigma = 1;
		final long stepRadius = 3;
		@SuppressWarnings( { "unchecked", "rawtypes" } )
		final HoughTransformOpNoWeights< BitType > hough = ( HoughTransformOpNoWeights ) Hybrids.unaryCF( ops,
				HoughTransformOpNoWeights.class,
				Img.class, img,
				minCircleRadius, maxCircleRadius, stepRadius );

		final Img< DoubleType > voteImg = hough.calculate( img );

		final ImagePlus imp = ImageJFunctions.show( img, "Two circles" );
		ImageJFunctions.show( voteImg, "Vote" );

		/*
		 * Hough detection.
		 */
			
		final double sensitivity = 1.;
		@SuppressWarnings( { "rawtypes", "unchecked" } )
		final HoughCircleDetectorDogOp< DoubleType > houghDetector = ( HoughCircleDetectorDogOp ) Functions.unary( ops,
				HoughCircleDetectorDogOp.class, 
				List.class, voteImg, 
				minCircleRadius, stepRadius, sigma, sensitivity );
		final List< HoughCircle > circles = houghDetector.calculate( voteImg );
		System.out.println( "Found " + circles.size() + " circles:" );
		imp.setOverlay( new Overlay() );
		for ( final HoughCircle circle : circles )
		{
			final OvalRoi roi = new OvalRoi(
					circle.getDoublePosition( 0 ) - circle.getRadius(),
					circle.getDoublePosition( 1 ) - circle.getRadius(),
					2. * circle.getRadius(),
					2. * circle.getRadius() );
			imp.getOverlay().add( roi );
			System.out.println( circle );
		}
		imp.updateAndDraw();

Maybe I did something wrong?

Gabe

@tinevez
Copy link
Member Author

tinevez commented Nov 4, 2017

Can I get the source image as well?

@gselzer
Copy link
Contributor

gselzer commented Feb 8, 2018

Tackling other PRs first, so it might be a while before I get back to this, but when I do:

  • Compare the output of this filter to that of the older version and the other older version
    We will decide how best to continue once the ops are compared against each other.

@gselzer
Copy link
Contributor

gselzer commented Mar 1, 2018

@tinevez I finally got around to working again on this PR and I started by comparing the results of this PR with the results of the UCB Hough Circle Transform plugin I have attached the result of a test on the same image as above.

hough_old_test_3

As you can see, it was able to find 16 circles (colored white and centered around the crosshair, with the unfound circles in gray), 4 more than the LocalExtrema Version of our newer Op. I am still getting the same results with the DoG Detector as before (see above).

Is it worth putting in the time at this point to improve this new Op when we have a plugin that works so well? Maybe it would be more worthwhile to refactor this older plugin into an op instead?

@gselzer gselzer force-pushed the hough-circle-transform branch from 2d37cd5 to 52c9513 Compare April 5, 2018 19:04
The transform is:
- 2D version only.
- weigthed or not.

The detectors retrieve the Hough circles from a vote image
generated by a HoughCircleTransformOp. The circles are
returned as a collection of HoughCircles.

Two versions:
- one with a DoG detector and sub-pixel accuracy.
- one with plain local extrema.

Signed-off-by: Curtis Rueden <ctrueden@wisc.edu>
@ctrueden ctrueden force-pushed the hough-circle-transform branch from 52c9513 to 934f45d Compare April 5, 2018 20:00
@gselzer
Copy link
Contributor

gselzer commented Apr 5, 2018

@ctrueden and I have cleaned up history and rebased over the latest master.

@tinevez the issues with detection quality still exist. I am moving on to other projects for now, this branch can sit until someone has time to look at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants