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

[Bill Lorensen]

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