[Insight-developers] BUG in BinaryBallStructuringElement
Padfield, Dirk R (GE Global Research)
padfield at research.ge.com
Thu Oct 17 10:18:59 EDT 2013
Hi Brad,
Thanks for your thoughts. You are right that some tests will fail with this change because of a slightly different definition of the structuring element. I looked through the tests that fail in the ITK tree, and all of them show a difference on the contour of the object that is about a pixel wide. This is expected because the new structuring element is slightly thinner than before. This is on the order of the difference between 4-connected and 8-connected neighborhood definition.
Other than these differences, are you concerned about other issues?
Thanks,
Dirk
________________________________________
From: Bradley Lowekamp [brad at lowekamp.net]
Sent: Wednesday, October 16, 2013 5:11 PM
To: Bill Lorensen
Cc: Padfield, Dirk R (GE Global Research); insight-developers at itk.org
Subject: Re: [Insight-developers] BUG in BinaryBallStructuringElement
I think it is a bad idea to change this. It'll likely introduce in consistencies with the different optimized algorithms.
> On Oct 16, 2013, at 3:34 PM, Bill Lorensen <bill.lorensen at gmail.com> wrote:
>
> Sounds like a rational argument to me. I suggest you submit a gerrit patch.
>
> Bill
>
>
> On Wed, Oct 16, 2013 at 3:31 PM, Padfield, Dirk R (GE Global Research)
> <padfield at research.ge.com> wrote:
>> Hi Bill,
>>
>> Thanks a lot for your feedback! In this case, the justification is based on the parametric definition of circles and ellipses where the diameter (axes) is twice the radius in each dimension. The "Size" of the structuring element is an entirely separate parameter; the Size of the kernel could be much larger than the size of the circle and still be valid (albeit with a lot of zeros in it!) although this would lead to a lot of unnecessary computation. I see this as being the same as the difference between the "Variance" and the "MaximumKernelWidth" of the GaussianImageFilter: the Variance is a parametric description whereas the MaximumKernelWidth defines the size of the kernel that holds that Gaussian. It just so happens that, in this case, the Size (2*radius + 1) was used to represent the ellipse/circle diameter although it should instead have been twice the radius.
>>
>> Does that help?
>>
>> Thanks,
>> Dirk
>>
>>
>> ________________________________________
>> From: Bill Lorensen [bill.lorensen at gmail.com]
>> Sent: Wednesday, October 16, 2013 3:12 PM
>> To: Padfield, Dirk R (GE Global Research)
>> Cc: insight-developers at itk.org
>> Subject: Re: [Insight-developers] BUG in BinaryBallStructuringElement
>>
>> Dirk,
>>
>> Is there anything in the literature that we can use to validate this?
>> Your logic seems correct to me.
>>
>> Bill
>>
>>
>> On Wed, Oct 16, 2013 at 2:05 PM, Padfield, Dirk R (GE Global Research)
>> <padfield at research.ge.com> wrote:
>>> Hi All,
>>>
>>> I am writing to ask your advice about a bug I found in BinaryBallStructuringElement.
>>>
>>> For a while, I have been bothered by the fact that the BinaryBallStructuringElement return balls that are larger than the specified radius. For example, when given a radius of 1, it returns the structuring element:
>>> 1 1 1
>>> 1 1 1
>>> 1 1 1
>>>
>>> But this structuring element has a radius that is more than 1! If it truly had a radius of 1, it would be a cross shape in this case.
>>>
>>> When choosing a larger radius, the problem is more obvious. Setting radius = 5 (leading to a structuring element size of 11x11) results in:
>>> 0 0 0 1 1 1 1 1 0 0 0
>>> 0 0 1 1 1 1 1 1 1 0 0
>>> 0 1 1 1 1 1 1 1 1 1 0
>>> 1 1 1 1 1 1 1 1 1 1 1
>>> 1 1 1 1 1 1 1 1 1 1 1
>>> 1 1 1 1 1 1 1 1 1 1 1
>>> 1 1 1 1 1 1 1 1 1 1 1
>>> 1 1 1 1 1 1 1 1 1 1 1
>>> 0 1 1 1 1 1 1 1 1 1 0
>>> 0 0 1 1 1 1 1 1 1 0 0
>>> 0 0 0 1 1 1 1 1 0 0 0
>>>
>>> This is clearly not an ellipse/circle with radius 5 because the interior ellipse/circle is touching each image border at five points rather than just one. As it turns out, the code is actually defining a radius that is "X + 0.5", where X is the radius that is requested!
>>>
>>> The problem is in the specification of the ellipse axes on lines 70-76 of itkBinaryBallStructuringElement.hxx:
>>> // Define and set the axes lengths for the ellipsoid
>>> typename EllipsoidType::InputType axes;
>>> for ( i = 0; i < VDimension; i++ )
>>> {
>>> axes[i] = this->GetSize(i);
>>> }
>>> spatialFunction->SetAxes(axes);
>>>
>>> In this case, "this->GetSize()" is equal to radius*2+1. But, an ellipse/circle with radius X should have axes length 2X, not 2X+1! In the implementation, the center of the ellipse is properly accounted for by setting it to "this->GetRadius+1", but the size of the ellipse is not correct!
>>>
>>> To correct this, we can make a simple change either
>>> axes[i] = this->GetSize(i) - 1;
>>> or
>>> axes[i] = this->GetRadius(i) * 2;
>>>
>>> The second is probably more intuitive.
>>>
>>> With this fix, we get for radius=1:
>>> 0 1 0
>>> 1 1 1
>>> 0 1 0
>>>
>>> and for radius=5:
>>> 0 0 0 0 0 1 0 0 0 0 0
>>> 0 0 1 1 1 1 1 1 1 0 0
>>> 0 1 1 1 1 1 1 1 1 1 0
>>> 0 1 1 1 1 1 1 1 1 1 0
>>> 0 1 1 1 1 1 1 1 1 1 0
>>> 1 1 1 1 1 1 1 1 1 1 1
>>> 0 1 1 1 1 1 1 1 1 1 0
>>> 0 1 1 1 1 1 1 1 1 1 0
>>> 0 1 1 1 1 1 1 1 1 1 0
>>> 0 0 1 1 1 1 1 1 1 0 0
>>> 0 0 0 0 0 1 0 0 0 0 0
>>>
>>> This is a true circle with radius 5!
>>>
>>> My questions are:
>>> 1) Is anyone else bothered by this bug? I imagine that many users expect the corrected version and don't realize they are getting the incorrect one.
>>> 2) Do others agree with this fix?
>>> 3) Can we make this fix given the number of filters/applications that will change slightly as a result of this fix?
>>>
>>> Many thanks,
>>> Dirk
>>>
>>>
>>>
>>> _______________________________________________
>>> Powered by www.kitware.com
>>>
>>> Visit other Kitware open-source projects at
>>> http://www.kitware.com/opensource/opensource.html
>>>
>>> Kitware offers ITK Training Courses, for more information visit:
>>> http://kitware.com/products/protraining.php
>>>
>>> Please keep messages on-topic and check the ITK FAQ at:
>>> http://www.itk.org/Wiki/ITK_FAQ
>>>
>>> Follow this link to subscribe/unsubscribe:
>>> http://www.itk.org/mailman/listinfo/insight-developers
>>
>>
>>
>> --
>> Unpaid intern in BillsBasement at noware dot com
>
>
>
> --
> Unpaid intern in BillsBasement at noware dot com
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Kitware offers ITK Training Courses, for more information visit:
> http://kitware.com/products/protraining.php
>
> Please keep messages on-topic and check the ITK FAQ at:
> http://www.itk.org/Wiki/ITK_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://www.itk.org/mailman/listinfo/insight-developers
More information about the Insight-developers
mailing list