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

[Tom Vercauteren]

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.
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: itk-portable-round-2009-05-26.patch
Type: text/x-diff
Size: 12464 bytes
Desc: not available
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20090526/a286bf6b/attachment.patch>