[Insight-developers] Portable round - Is it still time to commit?

Tue May 26 08:19:20 EDT 2009

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
>>>
>>>
>>
>