[Insight-users] Image offset is giving bad pointer for large datasets (7Gb)

[Michael Jackson]

---
Mike Jackson                 www.bluequartz.net



On Jul 13, 2009, at 11:30 AM, Bradley Lowekamp wrote:

> Luis,
>
> 	This looks like an absolutely massive effort you are attempting  
> here. I have grepped through the iterator classes are there alot of  
> incorrect usages of long:
>
> itkImageRegionReverseConstIterator.h:    m_SpanEndOffset = this- 
> >m_BeginOffset - static_cast<long>(this->m_Region.GetSize()[0]);

So that is a "bug" and should be corrected. This effort will help get  
rid of those.. I hope. Turning on -Wall may help track those down a  
bit easier but with the size of the code base of ITK there is going to  
be a large effort to track those down.

>
> And then there are complex relations like this:
>
> itkSize.h:  typedef   unsigned long     SizeValueType;
> itkImageRegion.h:  typedef Size<  
> itkGetStaticConstMacro( ImageDimension ) >  SizeType;
> itkImageRegion.h:  typedef typename  
> SizeType::SizeValueType                  SizeValueType;
> itkImageRegion.h:  SizeValueType GetNumberOfPixels() const;
>
>
> I am not really sure how all these issues can be tracked down. From  
> the user list:
>
>
> On Jul 10, 2009, at 12:46 PM, Michael Jackson wrote:
>
>>
>> IMNSHO, ITK (and VTK by extension) should absolutely BAN the use of
>> "long" in their projects. Period. Too many avoidable bugs come up  
>> from
>> its use. If you want a 32 bit integer use int or the standard ansi  
>> int
>> type, if you want a 64 bit integer then use "long long int" or the
>> standard ansi 64 bit integer type.
>>   "long int" is just a mess waiting to happen. Maybe a rule should be
>> put into the KWStyle project to look for and flag the use of 'long'?
>
>
> Do we need to go as far as banning the use of long and unsigned long?
>
>
> Good Luck Luis,
> Brad

I stated that without fully understanding what the SizeValueType  
really represented. For this case I think Luis has settled on using  
ptrdiff_t instead of "long". But I _still_ stand by my opinion that  
any use of "long" is bound to have really bad subtle bugs which are  
hard to find and fix.


Mike Jackson

>
> On Jul 10, 2009, at 6:33 PM, Luis Ibanez wrote:
>
>> On recent realization:
>>
>>              "size_t"    is unsigned       :-/
>>
>> and we need the OffsetValueType to be signed,
>> since we use it to compute differences...
>>
>> It seems that what we need is the type
>>
>>                         "ptrdiff_t"
>>
>> which is supposed to represent the differences
>> between two pointers, and therefore should be
>> capable of measuring distances between any
>> two locations in memory.
>>
>> I'm now rerunning the Experimental with
>> "ptrdiff_t" instead of "size_t".
>>
>>
>>        Luis
>>
>> ------------------------------------------------------------------------------------------------------
>> On Fri, Jul 10, 2009 at 12:30 PM, Luis Ibanez <luis.ibanez at kitware.com 
>> > wrote:
>> Hi Kana,
>>
>> Thanks a lot for looking into this and sharing your findings.
>>
>> I just confirmed that in Windows 64bits, the "long" type is
>> only 4 bytes.
>>
>> Here is the program I used:
>>
>> #include <iostream>
>> #include "itkOffset.h"
>> #include "itkNumericTraits.h"
>>
>>
>> int main()
>> {
>>   unsigned long tt;
>>   std::cout << "size = " << sizeof(tt) << std::endl;
>>   tt  =  -1;
>>   std::cout << "tt = " << tt << std::endl;
>>
>>   typedef itk::Offset<3>   OffsetType;
>>   typedef OffsetType::OffsetValueType   OffsetValueType;
>>
>>   OffsetValueType  offsetValue;
>>
>>   std::cout << "sizeof(offsetValue) = " << sizeof( offsetValue ) <<  
>> std::endl;
>>
>>   offsetValue = itk::NumericTraits< OffsetValueType >::max();
>>
>>   std::cout << "OffsetValueType max() = " << offsetValue <<  
>> std::endl;
>>
>>   return EXIT_SUCCESS;
>> }
>>
>>
>> with this CMakeLists.txt file
>>
>>
>> CMAKE_MINIMUM_REQUIRED(VERSION 2.4)
>> IF(COMMAND CMAKE_POLICY)
>>   CMAKE_POLICY(SET CMP0003 NEW)
>> ENDIF(COMMAND CMAKE_POLICY)
>>
>>
>> PROJECT(64bitsTest)
>>
>> FIND_PACKAGE(ITK REQUIRED)
>> INCLUDE(${ITK_USE_FILE})
>>
>> ADD_EXECUTABLE(typesTest typesTest.cxx )
>>
>> TARGET_LINK_LIBRARIES(typesTest ITKCommon)
>>
>>
>> <ATT00001.txt>
>
> ========================================================
> Bradley Lowekamp
> Lockheed Martin Contractor for
> Office of High Performance Computing and Communications
> National Library of Medicine
> blowekamp at mail.nih.gov