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
