[Insight-developers] Re: Limitation in MetaIO causing problems initkNeuralNetworkFileReader/Writer

Stephen R. Aylward Stephen.Aylward at Kitware.com
Tue Aug 21 12:08:43 EDT 2007 

You are, of course, right.

The itkNeuralNetworkIOReader/Writer is incomplete, internally 
inconsistent, and non-standard with Metaio.

The problems are
1) Does not use the field "BinaryDataByteOrderMSB" - this field is used 
by MetaIO to designate the byte order of the machine that wrote the 
data.  It is neither set nor read by MetaIO
2) If the data is ascii, reads using a field record and if the data is 
binary it is read directly from the file.   Line 307 of 
itkNeuralNetworkFileReader.   As with other MetaIO data, should be read 
without the use of a field record, regardless of binary/ascii.
3) Because of 1 (and 2) it doesn't handle byte order correctly.   The 
missing code should somewhat resemble the following:
   if(m_BinaryDataByteOrderMSB != MET_SystemByteOrderMSB())
     {
   int eSize;
   MET_SizeOfType(m_ElementType, &eSize);
   switch(eSize)
     {
     default:
     case 0:
     case 1:
       {
       break;
       }
     case 2:
       {
       int i;
       for(i=0; i<rows*cols; i++)
         {
         ((MET_USHORT_TYPE *)weightmatrix.data_block)[i] =
               MET_ByteOrderSwapShort(((MET_USHORT_TYPE 
*)m_ElementData)[i]);
         }
       break;
       }
     case 4:
       {
       int i;
       for(i=0; i<rows*cols i++)
         {
         ((MET_UINT_TYPE *)weightmatrix.data_block)[i] =
               MET_ByteOrderSwapLong(((MET_UINT_TYPE *)m_ElementData)[i]);
         }
       break;
       }
     case 8:
       {
       int i;
       char* data = (char*)weightmatrix.data_block;
       for(i=0; i<rows*cols; i++)
         {
         MET_ByteOrderSwap8(data);
         data += 8;
         }
       break;
       }
     }
   m_BinaryDataByteOrderMSB = !m_BinaryDataByteOrderMSB;
   }


kent williams wrote:
> I noticed that you were a co-author of the PDF for the Insight Journal...
> 
> I'm checking in a bounds check to the metaUtils for MET_InitWritefield. This
> will prevent crashes.
> 
> The problem with endian-ness comes in the itkNeuralNetworkFileReader -- if
> the WeightValuesType is BINARY, it simply reads in a chunk to fill out the
> network, without regard for endianness.  It doesn't use metaAraray to read
> and write the data.
> 
> I'll try and study up on metaIO and see if I can't put some sanity into the
> itkNeuralNetworkFileReader/Writer.
> 
> On 8/21/07 9:53 AM, "Stephen R. Aylward" <Stephen.Aylward at Kitware.com>
> wrote:
> 
>> Hi,
>>
>> Actually - MetaIO does correctly handle byte ordering - there are
>> functions to do an explicit swap on binary data as well as functions
>> that swap only if needed to convert the binary data into the native byte
>> order for the machine.   Handles 64bits as well as 32 bits, etc.
>>
>> I suspect that the NN IO libraries need help.  They may be breaking the
>> MetaIO standard - not the users fault - really the library developers
>> fault...and the library came in part from my old lab at UNC :)  So...if
>> they are flawed...blame UNC :)   Actually, I should take at look at
>> them...just wish there were more hours in a day/week/lifetime...
>>
>> s
>>
>> kent williams wrote:
>>> I certainly am not proposing to break existing, released code.
>>>
>>> Looking at the MetIO classes more closely this morning, it appears that it
>>> already supports binary data output, which doesn't have the limitations that
>>> The ASCII record type has with respect to data size.
>>>
>>> Like the vnl_matlab classes, it needs to take into account machine
>>> endianness, which right now it ignores.
>>>
>>> So in contrast to yesterday evening, I'm no longer running around with my
>>> hair on fire ;-)
>>>
>>>
>>> On 8/21/07 6:12 AM, "Atwood, Robert C" <r.atwood at imperial.ac.uk> wrote:
>>>
>>>>  
>>>> I for one would not be too happy with any such breakage, The intent as I
>>>> understand (and agree with) is that the meta-header is human readable,
>>>> and sticking tons of data into the tags would break this design idea,
>>>> let alone code using it.
>>>>
>>>> I use it all the time and can easily write separate (.mhd) headers by
>>>> hand, or by little scripts, for 3rd party and legacy image formats/image
>>>> series formats, and thus I can avoid using import filter code. My vote
>>>> is to please maintain this design.
>>>>
>>>> Robert
>>>>
>>>>> -----Original Message-----
>>>>> From: 
>>>>> insight-developers-bounces+r.atwood=imperial.ac.uk at itk.org
>>>>> [mailto:insight-developers-bounces+r.atwood=imperial.ac.uk at itk
>>>>> .org] On Behalf Of Stephen R. Aylward
>>>>> Sent: 20 August 2007 23:20
>>>>> To: kent williams
>>>>> Cc: Hans Johnson; ITK
>>>>> Subject: [Insight-developers] Re: Limitation in MetaIO
>>>>> causing problems initkNeuralNetworkFileReader/Writer
>>>>>
>>>>> Seems like the metaio library is being misused.  The fields are only
>>>>> intended for holding tag/value pairs.   Reading/writing large
>>>>> amounts of 
>>>>> data should done directly to the file or to an external file.
>>>>>
>>>>> Look at how matrix, arrays, and images are read/written -
>>>>> first read the
>>>>> tag/value pairs that specify how much data should be
>>>>> read/written, and
>>>>> then directly read/write the data from the file after the tag/value
>>>>> pairs.  Also, it should support writing those arrays as ascii
>>>>> or binary 
>>>>> data.
>>>>>
>>>>> It could be done the way you suggested, but that was not the
>>>>> design for 
>>>>> metaIO.
>>>>>
>>>>> Hope this helps,
>>>>> Stephen
>>>>>
>>>>> kent williams wrote:
>>>>>> I logged this as a bug: http://www.itk.org/Bug/view.php?id=5545
>>>>>>
>>>>>> The bug report has more details, but there are 2
>>>>> limitations with MetaIO, in
>>>>>> the context of the NeuralNet I/O classes: The
>>>>> MET_FieldRecordType is a C
>>>>>> structure, not a C++ class, and it contains a value array
>>>>> with a fixed size
>>>>>> of 255.
>>>>>>
>>>>>> I'm testing a very quick and dirty fix -- simply enlarging
>>>>> the value array
>>>>>> so that Hans can move forward with his NeuralNetwork code
>>>>> testing*, but
>>>>>> MetaIO maybe deserves some new scrutiny.
>>>>>>
>>>>>> Given that it's been around for years in ITK, I can't think
>>>>> of what I'd do
>>>>>> that wouldn't break backwards compatibility. The obvious
>>>>> thing to do would
>>>>>> be to move MET_FieldRecordType to a C++ class, make the value array
>>>>>> resizable, etc.  Any code depending on the current state of
>>>>> play would
>>>>>> break.
>>>>>>
>>>>>>
>>>>>> * and putting a test in MET_InitWriteField so it won't
>>>>> scribble all over
>>>>>> memory.  There are, as I count them, 4 separate potential
>>>>> buffer overruns in
>>>>>> that function.
>>>>>>
>>>>>>
>>>>> -- 
>>>>> =============================================================
>>>>> Stephen R. Aylward, Ph.D.
>>>>> Chief Medical Scientist
>>>>> Kitware, Inc. - Chapel Hill Office
>>>>> http://www.kitware.com
>>>>> Phone: (518)371-3971 x300
>>>>> _______________________________________________
>>>>> Insight-developers mailing list
>>>>> Insight-developers at itk.org
>>>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>>>
>>>> _______________________________________________
>>>> Insight-developers mailing list
>>>> Insight-developers at itk.org
>>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>
> 
> 

-- 
=============================================================
Stephen R. Aylward, Ph.D.
Chief Medical Scientist
Kitware, Inc. - Chapel Hill Office
http://www.kitware.com
Phone: (518)371-3971 x300

More information about the Insight-developers mailing list