[Insight-developers] Significant changes coming to address bug 6558

[Michel Audette]

Hi Tom and Simon,

thanks for your kind replys. Luis and I are working on this issue currently
and will take your suggestions into consideration.

Moreover, I will have a related email shortly that relates to regression
testing implications of this fix.

Best wishes,

Michel

On Wed, May 6, 2009 at 10:01 AM, Tom Vercauteren
<tom.vercauteren at gmail.com>wrote:

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



-- 
Michel Audette, Ph.D.
R & D Engineer,
Kitware Inc.,
Chapel Hill, N.C.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20090506/f5da7374/attachment.htm>