[Insight-developers] ShrinkFilter - Bugs from double precision

[Bradley Lowekamp]

The committed changes seem to achieve all the goals and desired  
behavior everyone has been wanting for this class.

http://public.kitware.com/cgi-bin/viewcvs.cgi/Code/BasicFilters/itkShrinkImageFilter.txx?root=Insight&r1=1.58&r2=1.60

Any additional feedback or review of these changes would be  
appreciated while I still have things fresh in my mind.

Thanks,
Brad


On Apr 9, 2009, at 10:30 AM, Lowekamp, Bradley (NIH/NLM/LHC) [C] wrote:

> Thank you! That explains it!
>
> I am still going to convert what can to index integer arithmetic and  
> the remainder I will use your recommendation.
>
> Thanks,
> Brad
>
> On Apr 9, 2009, at 10:14 AM, Tom Vercauteren wrote:
>
>> Hi Bradley,
>>
>> This is most certainly related to TransformPhysicalPointToIndex  
>> being wrong:
>> http://www.itk.org/Bug/view.php?id=6558
>>
>> Instead of rounding to the nearest integer,
>> TransformPhysicalPointToIndex simply casts the coordinates so if your
>> continuous index is
>>  2.9999999999999999
>> the corresponding index will be
>>  2
>> and not
>>  3
>>
>> I usually stay away from TransformPhysicalPointToIndex and use
>> TransformPhysicalPointToContinuousIndex followed by a loop of
>> vnl_math_round.
>>
>> Tom
>>
>> On Thu, Apr 9, 2009 at 16:01, Bradley Lowekamp <blowekamp at mail.nih.gov 
>> > wrote:
>>> To look into why my changes caused problem, I compared the  
>>> computed index
>>> between the ImageBase::Transform and the my supposed optimized  
>>> algebraic
>>> equivalent based on integer index operation.
>>>
>>> http://public.kitware.com/cgi-bin/viewcvs.cgi/Code/BasicFilters/itkShrinkImageFilter.txx?root=Insight&r1=1.56&r2=1.57&sortby=date
>>>
>>> The following is some debug information I printed comparing the  
>>> two. Notice
>>> with the shrink factors 2 1 1, the current implementation repeats  
>>> the second
>>> index, even though there is no shrinking being performs. Clearly  
>>> this is a
>>> bug, and is the likely candidate for causing the segfault, when  
>>> computed in
>>> the GenerateInputRequestedRegion, and the regions don't match up  
>>> correctly
>>> with the optimized version.
>>>
>>> The other thing to notice is the third component of the computed  
>>> Origin.
>>> Even though the shrink factor is 1 for both cases, the origin  
>>> changes. I can
>>> not find a reason for this. Any thoughts?
>>>
>>> I am planning on committing the change with the matched  
>>> computation in the
>>> ThreadedGenerateData and GernerateInputRequestedRegion methods.
>>>
>>> ShrinkFactors: 4 2 1
>>>  Spacing: [2, 5.4, 7.5]
>>> Origin: [-33.35, -178.2, -179.25]
>>>
>>>
>>> ShrinkFactors: 2 1 1
>>> Spacing: [1, 2.7, 7.5]
>>> Origin: [-32, -178.2, -179.75]
>>>
>>>
>>> output         input            optimized input
>>> Index          Index           Index
>>> [0, 2, 0] ->  [0, 1, 0] not [0, 2, 0]
>>> [0, 3, 0] ->  [0, 3, 0]
>>> [0, 4, 0] ->  [0, 3, 0] not [0, 4, 0]
>>> [0, 5, 0] ->  [0, 5, 0]
>>> [0, 6, 0] ->  [0, 6, 0]
>>> [0, 7, 0] ->  [0, 7, 0]
>>> [0, 8, 0] ->  [0, 8, 0]
>>> [0, 9, 0] ->  [0, 8, 0] not [0, 9, 0]
>>> [0, 10, 0] -> [0, 10, 0]
>>> [0, 11, 0] -> [0, 11, 0]
>>> [0, 12, 0] -> [0, 12, 0]
>>> [0, 13, 0] -> [0, 12, 0] not [0, 13, 0]
>>> [0, 14, 0] -> [0, 14, 0]
>>> [0, 15, 0] -> [0, 15, 0]
>>> [0, 16, 0] -> [0, 16, 0]
>>> [0, 17, 0] -> [0, 17, 0]
>>> [0, 18, 0] -> [0, 17, 0] not [0, 18, 0]
>>> [0, 19, 0] -> [0, 19, 0]
>>> [0, 20, 0] -> [0, 20, 0]
>>> [0, 21, 0] -> [0, 21, 0]
>>> [0, 22, 0] -> [0, 22, 0]
>>> [0, 23, 0] -> [0, 22, 0] not [0, 23, 0]
>>> [0, 24, 0] -> [0, 24, 0]
>>> [0, 25, 0] -> [0, 25, 0]
>>> [0, 26, 0] -> [0, 26, 0]
>>> [0, 27, 0] -> [0, 27, 0]
>>> [0, 28, 0] -> [0, 28, 0]
>>> [0, 29, 0] -> [0, 29, 0]
>>> [0, 30, 0] -> [0, 30, 0]
>>> [0, 31, 0] -> [0, 31, 0]
>>> [0, 32, 0] -> [0, 32, 0]
>>>
>>>
>>> On Apr 8, 2009, at 4:56 PM, Luis Ibanez wrote:
>>>
>>>>
>>>> Hi Brad,
>>>>
>>>> I would seem that your commit:
>>>> http://www.cdash.org/CDash/viewUpdate.php?buildid=308797
>>>>
>>>> generated the following segmentation fault:
>>>> http://www.cdash.org/CDash/viewTest.php?onlyfailed&buildid=308797
>>>>
>>>> Could you please take a look ?
>>>>
>>>>
>>>>   Thanks
>>>>
>>>>
>>>>      Luis
>>>>
>>>
>>> _______________________________________________
>>> 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
>>>
>
> ========================================================
> Bradley Lowekamp
> Lockheed Martin Contractor for
> Office of High Performance Computing and Communications
> National Library of Medicine
> blowekamp at mail.nih.gov
>
>
> <ATT00001.txt>

========================================================
Bradley Lowekamp
Lockheed Martin Contractor for
Office of High Performance Computing and Communications
National Library of Medicine
blowekamp at mail.nih.gov


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20090410/57060cb3/attachment.htm>