<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
</head>
<body style="word-wrap:break-word; color:rgb(0,0,0); font-size:14px; font-family:Calibri,sans-serif">
<div>Dirk,</div>
<div><br>
</div>
<div>I'm concerned about the backwards compatibility issues. I would recommend that a new set of member functions for setting the radius be added that implements your solution, but leave the current API with no backwards compatibility issues.</div>
<div><br>
</div>
<div>Hans</div>
<div><br>
</div>
<span id="OLK_SRC_BODY_SECTION">
<div style="font-family:Calibri; font-size:11pt; text-align:left; color:black; border-bottom:medium none; border-left:medium none; padding-bottom:0in; padding-left:0in; padding-right:0in; border-top:#b5c4df 1pt solid; border-right:medium none; padding-top:3pt">
<span style="font-weight:bold">From: </span>Wes Turner <<a href="mailto:wes.turner@kitware.com">wes.turner@kitware.com</a>><br>
<span style="font-weight:bold">Date: </span>Thursday, October 17, 2013 8:51 AM<br>
<span style="font-weight:bold">To: </span>"Padfield, Dirk R (GE Global Research)" <<a href="mailto:padfield@research.ge.com">padfield@research.ge.com</a>><br>
<span style="font-weight:bold">Cc: </span>ITK <<a href="mailto:insight-developers@itk.org">insight-developers@itk.org</a>><br>
<span style="font-weight:bold">Subject: </span>Re: [Insight-developers] BUG in BinaryBallStructuringElement<br>
</div>
<div><br>
</div>
<div>
<div>
<div dir="ltr">Just to weigh in on this, I am finding this class in the repository since 2002. I agree that Dirk's proposal better approximates the true area, but I am not convinced it represents an error. The difference seems to be more one of interpretation.
Is a voxel in the structuring element if it is cut by the parametric ball, or only if its center is included in the parametric ball? This will break backwards compatibility for some users, is there enough consensus that the center-based interpretation is correct
and not just an alternative interpretation?
<div><br>
</div>
<div>- Wes </div>
</div>
<div class="gmail_extra"><br>
<br>
<div class="gmail_quote">On Thu, Oct 17, 2013 at 9:09 AM, Padfield, Dirk R (GE Global Research)
<span dir="ltr"><<a href="mailto:padfield@research.ge.com" target="_blank">padfield@research.ge.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex; border-left:1px #ccc solid; padding-left:1ex">
Hi Ho,<br>
<br>
Thanks for your feedback and insight! I agree that discretizing continuous functions is always a tricky thing. Luckily, we have the spatial objects to help with this since they define their own inside-outside tests. The Ellipse spatial object is used in
the BinaryBallStructuringElement implementation, but the problem is that the spatial object itself is used incorrectly. By definition, the axes should be "radius*2" rather than "radius*2+1". Defining the axes of an ellipse/circle to be "radius*2+1" is simply
an error.<br>
<br>
We can also attack this question by considering the area of the continuous function versus the discretized version by counting the number of "on" pixels in the kernel as follows:<br>
<br>
For radius=1, the true area is pi = 3.14<br>
Using the old version, we get 9<br>
Using the correction, we get 5<br>
<br>
For radius=5, the true area is 25*pi = 78.5<br>
Using the old version, we get 97 (24% error)<br>
Using the correction, we get 81 (3% error)<br>
<br>
For radius=11, the true area is 121*pi = 380<br>
Using the old version, we get 421 (11% error)<br>
Using the correction, we get 377 (1% error)<br>
<br>
For radius=21, the true area is 21*21*pi = 1385<br>
Using the old version, we get 1457 (5% error)<br>
Using the correction, we get 1373 (1% error)<br>
<br>
As expected, as the radius increases, the discretized version better approximates the continuous function. We can also see that the corrected version is always more accurate than the old version.<br>
<br>
What do you think?<br>
<br>
Thanks,<br>
Dirk<br>
<br>
<br>
________________________________<br>
From: Ho Cheung [<a href="mailto:hocheung20@gmail.com">hocheung20@gmail.com</a>]<br>
Sent: Wednesday, October 16, 2013 6:26 PM<br>
To: Padfield, Dirk R (GE Global Research)<br>
Cc: ITK<br>
<div class="im">Subject: Re: [Insight-developers] BUG in BinaryBallStructuringElement<br>
<br>
</div>
<div class="im">Dirk,<br>
<br>
As a counterpoint, I do not agree that there is a bug but rather just an ambiguity in the way we have defined whether or not a pixel is to be included.<br>
<br>
If you take a protractor and plotted a unit circle, then superimpose a grid on it (this this case, 3x3), and then shaded in the nearest pixels to the circle, it would look like the “original” example. The same applies to the radius 5 circle.<br>
<br>
Technically, if you look at the parametric definition of a circle, then yes, those pixels would not be included, as their physical space points fall outside the circle.<br>
<br>
However, I believe (anecdotal) in graphics rendering, it is common practice to include those pixels which are nearest to the actual physical space point.<br>
<br>
Regards,<br>
<br>
Ho Cheung<br>
<a href="tel:%28775%29%20388-2368" value="+17753882368">(775) 388-2368</a><br>
<br>
</div>
<div>
<div class="h5">On Oct 16, 2013, at 1:05 PM, Padfield, Dirk R (GE Global Research) <<a href="mailto:padfield@research.ge.com">padfield@research.ge.com</a><mailto:<a href="mailto:padfield@research.ge.com">padfield@research.ge.com</a>>> wrote:<br>
<br>
Hi All,<br>
<br>
I am writing to ask your advice about a bug I found in BinaryBallStructuringElement.<br>
<br>
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:<br>
1 1 1<br>
1 1 1<br>
1 1 1<br>
<br>
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.<br>
<br>
When choosing a larger radius, the problem is more obvious. Setting radius = 5 (leading to a structuring element size of 11x11) results in:<br>
0 0 0 1 1 1 1 1 0 0 0<br>
0 0 1 1 1 1 1 1 1 0 0<br>
0 1 1 1 1 1 1 1 1 1 0<br>
1 1 1 1 1 1 1 1 1 1 1<br>
1 1 1 1 1 1 1 1 1 1 1<br>
1 1 1 1 1 1 1 1 1 1 1<br>
1 1 1 1 1 1 1 1 1 1 1<br>
1 1 1 1 1 1 1 1 1 1 1<br>
0 1 1 1 1 1 1 1 1 1 0<br>
0 0 1 1 1 1 1 1 1 0 0<br>
0 0 0 1 1 1 1 1 0 0 0<br>
<br>
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!<br>
<br>
The problem is in the specification of the ellipse axes on lines 70-76 of itkBinaryBallStructuringElement.hxx:<br>
// Define and set the axes lengths for the ellipsoid<br>
typename EllipsoidType::InputType axes;<br>
for ( i = 0; i < VDimension; i++ )<br>
{<br>
axes[i] = this->GetSize(i);<br>
}<br>
spatialFunction->SetAxes(axes);<br>
<br>
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!<br>
<br>
To correct this, we can make a simple change either<br>
axes[i] = this->GetSize(i) - 1;<br>
or<br>
axes[i] = this->GetRadius(i) * 2;<br>
<br>
The second is probably more intuitive.<br>
<br>
With this fix, we get for radius=1:<br>
0 1 0<br>
1 1 1<br>
0 1 0<br>
<br>
and for radius=5:<br>
0 0 0 0 0 1 0 0 0 0 0<br>
0 0 1 1 1 1 1 1 1 0 0<br>
0 1 1 1 1 1 1 1 1 1 0<br>
0 1 1 1 1 1 1 1 1 1 0<br>
0 1 1 1 1 1 1 1 1 1 0<br>
1 1 1 1 1 1 1 1 1 1 1<br>
0 1 1 1 1 1 1 1 1 1 0<br>
0 1 1 1 1 1 1 1 1 1 0<br>
0 1 1 1 1 1 1 1 1 1 0<br>
0 0 1 1 1 1 1 1 1 0 0<br>
0 0 0 0 0 1 0 0 0 0 0<br>
<br>
This is a true circle with radius 5!<br>
<br>
My questions are:<br>
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.<br>
2) Do others agree with this fix?<br>
3) Can we make this fix given the number of filters/applications that will change slightly as a result of this fix?<br>
<br>
Many thanks,<br>
Dirk<br>
<br>
<br>
<br>
_______________________________________________<br>
</div>
</div>
Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><<a href="http://www.kitware.com" target="_blank">http://www.kitware.com</a>><br>
<div class="HOEnZb">
<div class="h5"><br>
Visit other Kitware open-source projects at<br>
<a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Kitware offers ITK Training Courses, for more information visit:<br>
<a href="http://kitware.com/products/protraining.php" target="_blank">http://kitware.com/products/protraining.php</a><br>
<br>
Please keep messages on-topic and check the ITK FAQ at:<br>
<a href="http://www.itk.org/Wiki/ITK_FAQ" target="_blank">http://www.itk.org/Wiki/ITK_FAQ</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://www.itk.org/mailman/listinfo/insight-developers" target="_blank">http://www.itk.org/mailman/listinfo/insight-developers</a><br>
<br>
_______________________________________________<br>
Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at<br>
<a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Kitware offers ITK Training Courses, for more information visit:<br>
<a href="http://kitware.com/products/protraining.php" target="_blank">http://kitware.com/products/protraining.php</a><br>
<br>
Please keep messages on-topic and check the ITK FAQ at:<br>
<a href="http://www.itk.org/Wiki/ITK_FAQ" target="_blank">http://www.itk.org/Wiki/ITK_FAQ</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://www.itk.org/mailman/listinfo/insight-developers" target="_blank">http://www.itk.org/mailman/listinfo/insight-developers</a><br>
</div>
</div>
</blockquote>
</div>
<br>
<br clear="all">
<div><br>
</div>
-- <br>
Wesley D. Turner, Ph.D.<br>
Kitware, Inc.<br>
Technical Leader<br>
28 Corporate Drive<br>
Clifton Park, NY 12065-8662<br>
Phone: 518-881-4920 </div>
</div>
</div>
</span><br>
<br>
<hr>
Notice: This UI Health Care e-mail (including attachments) is covered by the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is confidential and may be legally privileged. If you are not the intended recipient, you are hereby notified that any
retention, dissemination, distribution, or copying of this communication is strictly prohibited. Please reply to the sender that you have received the message in error, then delete it. Thank you.
<hr>
</body>
</html>