[Insight-developers] Portable round - Is it still time to commit?
Michel Audette
michel.audette at kitware.com
Tue May 26 08:28:17 EDT 2009
Nice recovery, Tom.
I don't know if this is the case yet, but I'm thinking that unit testing
should alternate between data that has even and odd dimensions. Perhaps a
pattern emerges sometime when there is a bias in failing tests towards one
or the other.
Cheers,
Michel
On Tue, May 26, 2009 at 8:19 AM, Bill Lorensen <bill.lorensen at gmail.com>wrote:
> No problem. The repository is frozen. Wes or Luis will have to make
> the changes to itkMacro.h.
>
> On Tue, May 26, 2009 at 7:43 AM, Tom Vercauteren
> <tom.vercauteren at m4x.org> wrote:
> > Hi Bill,
> >
> > It seems that I am the culprit :(
> > This apparently stems from the patch I proposed on the bug tracker. I
> > am really really sorry for the mess. This is definitely the result of
> > insufficient unit testing, I am preparing a test for itk::Math::Round,
> > it will mimic the test from vnl_math. I'll post it ASAP.
> >
> > Tom
> >
> > On Tue, May 26, 2009 at 13:37, Bill Lorensen <bill.lorensen at gmail.com>
> wrote:
> >> Tom,
> >>
> >> I believe that you are correct and that the consensus was to round up.
> This
> >> inline int Round(float x) { return RoundHalfIntegerUp(x); }
> >> inline int Round(double x) { return RoundHalfIntegerUp(x); }
> >>
> >> should be fixed before tagging. Looks like we'll need another day if
> >> it's not too late.
> >>
> >> Bill
> >>
> >> On Tue, May 26, 2009 at 4:53 AM, Tom Vercauteren
> >> <tom.vercauteren at m4x.org> wrote:
> >>> Hi all,
> >>>
> >>> Guess I did the code review of ITK_USE_PORTABLE_ROUND a little too
> >>> fast. I didn't spot another problem with the portable rounding
> >>> function.
> >>>
> >>> I was so sure that it::Math::Round was rounding half-integers upward
> >>> that I read it that way in itkMacro.h. Actually, what we have in this
> >>> file is
> >>>
> >>> inline int Round(float x) { return RoundHalfIntegerToEven(x); }
> >>> inline int Round(double x) { return RoundHalfIntegerToEven(x); }
> >>>
> >>> instead of what I understood the consensus would have led to
> >>>
> >>> inline int Round(float x) { return RoundHalfIntegerUp(x); }
> >>> inline int Round(double x) { return RoundHalfIntegerUp(x); }
> >>>
> >>> Changing this fixes the two additional failing tests that appeared
> >>> with the previous patch (ResampleImageFilter3Test and
> >>> ResampleImageFilter9Test) when
> >>> ITK_USE_CENTERED_PIXEL_COORDINATES_CONSISTENTLY was off.
> >>>
> >>> For your consideration, I have attached an updated patch. Also as
> >>> shown in this patch, there is a small cmake issue in
> >>> ResampleImageFilter3Test1 when
> >>> ITK_USE_CENTERED_PIXEL_COORDINATES_CONSISTENTLY is on.
> >>>
> >>> Hope this helps,
> >>>
> >>> Tom
> >>>
> >>>
> >>> On Tue, May 26, 2009 at 02:02, Michel Audette
> >>> <michel.audette at kitware.com> wrote:
> >>>> Hi gents,
> >>>>
> >>>> I've isolated the 2 failing tests to something in itkMacro.h part of
> the
> >>>> patch.
> >>>>
> >>>> Cheers,
> >>>>
> >>>> Michel
> >>>>
> >>>> On Mon, May 25, 2009 at 4:47 PM, Luis Ibanez <luis.ibanez at kitware.com
> >
> >>>> wrote:
> >>>>>
> >>>>> Thanks,
> >>>>>
> >>>>>
> >>>>> Luis
> >>>>>
> >>>>> ------------------------------
> >>>>> On Mon, May 25, 2009 at 4:42 PM, Michel Audette
> >>>>> <michel.audette at kitware.com> wrote:
> >>>>>>
> >>>>>> Thanks for pointing that out. I missed those. Let me try a clean
> build
> >>>>>> again then.
> >>>>>>
> >>>>>> Michel
> >>>>>>
> >>>>>> On Mon, May 25, 2009 at 4:40 PM, Luis Ibanez <
> luis.ibanez at kitware.com>
> >>>>>> wrote:
> >>>>>>>
> >>>>>>> Hi Michel,
> >>>>>>>
> >>>>>>> Please note that the submission with two failing tests,
> >>>>>>> has link errors:
> >>>>>>>
> >>>>>>> http://www.cdash.org/CDash/viewBuildError.php?buildid=340518
> >>>>>>>
> >>>>>>> We may not want to draw conclusions about failing/passing
> >>>>>>> tests until the build itself is clean of errors.
> >>>>>>>
> >>>>>>>
> >>>>>>> Luis
> >>>>>>>
> >>>>>>>
> >>>>>>>
> --------------------------------------------------------------------
> >>>>>>> On Mon, May 25, 2009 at 4:37 PM, Michel Audette
> >>>>>>> <michel.audette at kitware.com> wrote:
> >>>>>>>>
> >>>>>>>> Hi Luis,
> >>>>>>>>
> >>>>>>>> I'm trying to find out more. I went back to the presently
> committed
> >>>>>>>> version of Insight, and ascertained that no ctest -V Resample does
> not fail
> >>>>>>>> there.
> >>>>>>>>
> >>>>>>>> I'm currently adding each of Tom's patched files one by one,
> compiling
> >>>>>>>> and trying to determine which of these is the culprit. I don't
> know of a
> >>>>>>>> quicker way to do it.
> >>>>>>>>
> >>>>>>>> Feel free to call me if you want to discuss things.
> >>>>>>>>
> >>>>>>>> Cheers,
> >>>>>>>>
> >>>>>>>> Michel
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On Mon, May 25, 2009 at 4:33 PM, Luis Ibanez <
> luis.ibanez at kitware.com>
> >>>>>>>> wrote:
> >>>>>>>>>
> >>>>>>>>> Hi Michel,
> >>>>>>>>>
> >>>>>>>>> Why are these two test not showing up
> >>>>>>>>> as failings in the Continuous builds ?
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Thanks for any hint,
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Luis
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> -------------------------------------------------
> >>>>>>>>> On Mon, May 25, 2009 at 3:05 PM, Michel Audette
> >>>>>>>>> <michel.audette at kitware.com> wrote:
> >>>>>>>>>>
> >>>>>>>>>> Hi Tom,
> >>>>>>>>>>
> >>>>>>>>>> you were bang on. I have done a make clean and re-built the
> code, and
> >>>>>>>>>> it looks like it's doing better.
> >>>>>>>>>>
> >>>>>>>>>> On another note, two tests appear to fail with the Portable
> Round,
> >>>>>>>>>> Pixel-centered, and Region-validation flags off.
> >>>>>>>>>>
> >>>>>>>>>>
> http://www.cdash.org/CDash/viewTest.php?onlyfailed&buildid=340518
> >>>>>>>>>>
> >>>>>>>>>> These two tests also were failing with the flags on. Is there a
> part
> >>>>>>>>>> of the new code that is not #ifdef'ed with the flags?
> >>>>>>>>>>
> >>>>>>>>>> The ctest -Experimental with flags on is still running.
> >>>>>>>>>>
> >>>>>>>>>> Cheers,
> >>>>>>>>>>
> >>>>>>>>>> Michel
> >>>>>>>>>>
> >>>>>>>>>> On Mon, May 25, 2009 at 2:05 PM, Tom Vercauteren
> >>>>>>>>>> <tom.vercauteren at gmail.com> wrote:
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> _______________________________________________
> >>>>>>>>>> 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.
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> --
> >>>>>> Michel Audette, Ph.D.
> >>>>>> R & D Engineer,
> >>>>>> Kitware Inc.,
> >>>>>> Chapel Hill, N.C.
> >>>>>>
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>> --
> >>>> 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/20090526/7b3cfc2c/attachment.htm>
More information about the Insight-developers
mailing list