-
Notifications
You must be signed in to change notification settings - Fork 42
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
base: master
Are you sure you want to change the base?
Conversation
Yes my bad. |
Hey @tinevez! 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.
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, |
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.
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 am not too sure about this. Normally you would threshold the edge image (defined over 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.
Yes this one the weak point here. The output of the detection process is a custom class called |
@gselzer wrote:
Here is a Groovy script which binarizes an image using the ImgLib2 #@ 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. |
@tinevez wrote:
I suppose you are right about separation of concerns. I still think that the convenience that the op would give by using
I don't have any strong feelings one way or the other. We are going to have to either refactor the whole of |
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: Here is the result from LocalExtrema, with no params changed :
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
@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 |
Hi @gselzer
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? |
There was a problem hiding this 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." ); } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok.
There was a problem hiding this comment.
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 callsops().op()
, which would return an Op reference that the user could use to run through a bunch of images, instead of callingops().run()
with every image. While the user could theoretically pass an image into this reference that would not be caught byconforms
, 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.
There was a problem hiding this comment.
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." ); } |
There was a problem hiding this comment.
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 ); |
There was a problem hiding this comment.
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 ); |
There was a problem hiding this comment.
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( |
There was a problem hiding this comment.
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 ) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 ); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
Hi @gselzer I chose not to rely on the |
Hey @tinevez! So I think with 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):
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? |
Hi @gselzer |
@tinevez Here is the code I used.
Maybe I did something wrong? Gabe |
Can I get the source image as well? |
Tackling other PRs first, so it might be a while before I get back to this, but when I do:
|
@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. 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? |
2d37cd5
to
52c9513
Compare
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>
52c9513
to
934f45d
Compare
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...)
Output: