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 [[email protected]] Sent: Wednesday, October 16, 2013 5:11 PM To: Bill Lorensen Cc: Padfield, Dirk R (GE Global Research); [email protected] 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 <[email protected]> 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) > <[email protected]> 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 [[email protected]] >> Sent: Wednesday, October 16, 2013 3:12 PM >> To: Padfield, Dirk R (GE Global Research) >> Cc: [email protected] >> 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) >> <[email protected]> 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 _______________________________________________ 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
