Hi Tom and Simon, <br><br>thanks for your kind replys. Luis and I are working on this issue currently and will take your suggestions into consideration. <br><br>Moreover, I will have a related email shortly that relates to regression testing implications of this fix. <br>
<br>Best wishes, <br><br>Michel<br><br><div class="gmail_quote">On Wed, May 6, 2009 at 10:01 AM, Tom Vercauteren <span dir="ltr"><<a href="mailto:tom.vercauteren@gmail.com">tom.vercauteren@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">Hi Michel,<br>
<br>
It would be great to see such a fix in ITK 3.14! Here's a follow-up on<br>
Simon's comments.<br>
<br>
<br>
Point A<br>
-----------<br>
I agree with Simon that we should try to handle all interpolators in a<br>
consistent manner. One way to do so could be to add extrapolation<br>
capabilities to the interpolators. What needs to be done for this is<br>
relatively easy. An example is given here:<br>
<a href="http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Review/itkVectorLinearInterpolateNearestNeighborExtrapolateImageFunction.txx?root=Insight&sortby=date&view=markup" target="_blank">http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Review/itkVectorLinearInterpolateNearestNeighborExtrapolateImageFunction.txx?root=Insight&sortby=date&view=markup</a><br>
<br>
Basically, the only thing it does is, within<br>
EvaluateAtContinuousIndex, modify the input continuous index to be the<br>
closest continuous index that is strictly inside the region (using the<br>
convention you use, i.e. not in the half pixel border).<br>
<br>
Also, this approach has the nice side effect of allowing the<br>
implementation of<br>
XXXInterpolateNearestNeighborExtrapolateImageFunction for almost free.<br>
One only needs to either not check for IsInsideBuffer or inherit from<br>
XXXInterpolateImageFunction and override IsInsideBuffer to always<br>
return true.<br>
<br>
Technically the easiest might be to factorize the modification of the<br>
input index within a small function of InterpolateImageFunction (or<br>
ImageFunction). Maybe something like<br>
ContinuousIndexType GetClosestStrictlyInsideContinuousIndex( const<br>
ContinuousIndexType & )<br>
<br>
Note that this might not be the best scheme for higher order<br>
interpolators (e.g. BSplines) since at the border, they might do a<br>
little better than NN extrapolation. However, there is no other<br>
generic way I can think of for handling this. Also NN is already a<br>
decent border condition.<br>
<br>
<br>
Point B<br>
-----------<br>
For the vector interpolators, I also think that they are almost<br>
useless if we rewrite the code a little bit. For example, all it took<br>
for the linear interpolator to handle vector images is here:<br>
<a href="http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Common/itkLinearInterpolateImageFunction.txx?root=Insight&r1=1.41&r2=1.37&sortby=date" target="_blank">http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Common/itkLinearInterpolateImageFunction.txx?root=Insight&r1=1.41&r2=1.37&sortby=date</a><br>
with the unit test<br>
<a href="http://www.itk.org/cgi-bin/viewcvs.cgi/Testing/Code/Common/itkLinearInterpolateImageFunctionTest.cxx?root=Insight&r1=1.3&r2=1.4&sortby=date" target="_blank">http://www.itk.org/cgi-bin/viewcvs.cgi/Testing/Code/Common/itkLinearInterpolateImageFunctionTest.cxx?root=Insight&r1=1.3&r2=1.4&sortby=date</a><br>
<br>
By the way, the same thing should be true for complex images. I just<br>
saw these classes:<br>
<a href="http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Review/itkComplexBSplineInterpolateImageFunction.h?root=Insight&view=markup" target="_blank">http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Review/itkComplexBSplineInterpolateImageFunction.h?root=Insight&view=markup</a><br>
<a href="http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Review/itkComplexBSplineInterpolateImageFunction.txx?root=Insight&view=markup" target="_blank">http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Review/itkComplexBSplineInterpolateImageFunction.txx?root=Insight&view=markup</a><br>
<br>
I don't think we need them. BSplineInterpolateImageFunction should be<br>
able to handle complex pixels as good as vectors or (real) scalars.<br>
<br>
<br>
Point C<br>
-----------<br>
Sorry for the use of vnl_math_round in my initial patch. It should<br>
have been vnl_math_rnd from the beginning (vnl_math_round has never<br>
existed). This doesn't raise any error (on my computer) because this<br>
is in a piece of code that is not compiled when loop unrolling is<br>
turned on (which is the default).<br>
<br>
That being said, as mentioned by Simon, I think we need to move away<br>
from vnl_math_rnd and use something like vnl_math_rnd_halfintup<br>
<a href="https://apps.sourceforge.net/trac/vxl/ticket/23" target="_blank">https://apps.sourceforge.net/trac/vxl/ticket/23</a><br>
throughout ITK.<br>
<br>
<br>
Cheers,<br>
Tom<br>
<div><div></div><div class="h5"><br>
<br>
On Tue, May 5, 2009 at 23:57, Michel Audette <<a href="mailto:michel.audette@kitware.com">michel.audette@kitware.com</a>> wrote:<br>
> Dear members of the Insight community,<br>
><br>
> we are currently at work to solve bug 6558: Physical coordinates of a pixel<br>
> - Severe inconsistency and bug in ImageBase. We've found that the<br>
> implementation of pixel-centered positions had unintended and adverse<br>
> effects on other classes, which we have also been correcting for the past<br>
> two days. This fix appears to be a priority of many members of the<br>
> community, so we feel it's important to provide the required functionality<br>
> integrally.<br>
><br>
> We will make available a patch to the Insight community to apply and comment<br>
> on.<br>
> This patch can be found at: <a href="http://public.kitware.com/Bug/view.php?id=6558" target="_blank">http://public.kitware.com/Bug/view.php?id=6558</a>.<br>
> Feel free to comment on its validity either by email or in the SecondLife<br>
> discussions on Friday afternoons.<br>
><br>
> We plan on making the fix permanent in the upcoming release.<br>
><br>
> Best wishes,<br>
><br>
> Michel<br>
><br>
> --<br>
> Michel Audette, Ph.D.<br>
> R & D Engineer,<br>
> Kitware Inc.,<br>
> Chapel Hill, N.C.<br>
><br>
><br>
</div></div>> _______________________________________________<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>
> 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>
</blockquote></div><br><br clear="all"><br>-- <br>Michel Audette, Ph.D. <br>R & D Engineer, <br>Kitware Inc.,<br>Chapel Hill, N.C. <br><br>