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

Tue May 26 07:43:00 EDT 2009

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