[Insight-developers] Image memory model

[Iván Macía]

Dear all, 

I added a few comments and a new section with comments on Filters.

Best regards

Ivan

-----Mensaje original-----
De: Luis Ibanez [mailto:luis.ibanez at kitware.com] 
Enviado el: viernes, 30 de mayo de 2008 15:15
Para: Iván Macía
CC: 'Dan Mueller'; 'Karthik Krishnan'; 'Insight Developers'
Asunto: Re: [Insight-developers] Image memory model


Dan, Ivan, Karthik,

There is a Wiki page dedicated to this discussion at:
http://www.itk.org/Wiki/Proposals:Slice_contiguous_images

In the table of proposals from 2005:
http://www.itk.org/Wiki/ITK_Oversight_Committee#2005

Could you please use this Wiki page in order to gather
your observations from this discussion ?

(in addition to follow the discussion on the developers list).

and if you want to run experimental code, it will be great
to put it in the NAMIC Sandbox directory:
http://www.na-mic.org/svn/NAMICSandBox/trunk/SliceBasedImageMemoryLayout/

Once a group of SliceBaseImage classes are crafted there, the
next step will be to submit them to the Insight Journal (along
with testing), so that they can enter in the process of being
added to the toolkit.


Please let us know if you encounter any problems,


    Thanks



-----------------
Iván Macía wrote:
> Dear all,
> 
> I have been following this discussion and I'm particularly interested on
it.
> 
> 
> We are currently developing a medical image viewer application. It must
> support images of almost any type and size. For this is very important to
> use a slice-contiguous image memory model, because the software should run
> in a PACS system and should be virtually scalable to any image size.
> 
> This implies that only a limited amount of slices are kept on memory at
any
> time and that images are cached on disk and should be loaded on demand.
For
> example images are transferred to a local directory from the network and
> they load as they become available up to a maximum amount of slices. The
> user, for example, could use a slider in a multi-slice viewer and then
some
> other available images could be loaded and displayed.
> 
> What we did so far is using a combination of Series/File Readers together
> with simple container of standard ITK images (i.e.
std::vector<itk::Image<>
> 
>>). However, it would be much better if this memory model could be
> 
> integrated into the core ITK so it could be used seamlessly together with
> the rest of ITK images. Moreover, in my opinion, this slice-contiguous
model
> should work in combination with Readers/Writers with streaming
capabilities
> (do the GDCMSeriesReaders/Writers already allow this? it seems like
> something is done). I have had a look at the implementation of the NAMIC
> SandBox and I think it is on the right way, but of course, if it could be
> connected to a streamable Reader/Writers.
> 
> Regarding the filters, such an application, definitely would use some
> filters, usually simple intensity or histogram-based filter that could
work
> easily work with this memory model. 
> 
> Some other filters, such as neighborhood filters, could work if a smart
> caching strategy devised so all needed slices are on memory when required
> for the filter (even in loading and processing in separated but
synchronized
> threads?).
> 
> Finally there are some filters that simply won't work with their current
> implementation. As someone pointed out, maybe ConceptChecking could be the
> solution here.
> 
> These are just some ideas. I don't know if this could be in the future
plans
> of ITK but it would be really very interesting to have.
> 
> Best regards
> 
> Iván Macía
> 
> -----Mensaje original-----
> De: insight-developers-bounces at itk.org
> [mailto:insight-developers-bounces at itk.org] En nombre de Dan Mueller
> Enviado el: viernes, 30 de mayo de 2008 6:03
> Para: Karthik Krishnan
> CC: Insight Developers
> Asunto: Re: [Insight-developers] Image memory model
> 
> Hi Karthik (and developers),
> 
> Thanks for your detailed response regarding the implementation of a
> slice contiguous image. Sorry I have taken so long to write back, but
> it has been hard to find the time to work on this issue.
> 
> Regarding the design of the pixel accessors: you're right, it's not
> "too good to be true", it is just true! What a wonderful design!
> 
> I have used your ideas and now have a working
> itk::SliceContiguousImage into which I can import slices from
> non-contiguous memory for use with linear iterators (and filters which
> only use linear iterators). I still need to polish up the API and
> think about the neighborhood accessor, but things are looking good!
> 
> A few further questions:
> 
> 1. What should SliceContiguousImage do with GetBufferPointer()?
> In my current design, GetBufferPointer() simply returns 0. I guess
> this is logical because there is no "buffer", rather an array of
> pointers to slices -- currently stored as std::vector< PixelType * >.
> With this design the new SliceContiguousPixelAccessor must be given
> access to both the array of slices and the buffered region size. I
> think this could be improved if GetBufferPointer() were to return the
> pointer to the list of slices rather than 0. Do you think this is too
> misleading (the name of the method no longer describes fully what is
> being returned)? If it is not too misleading, then the only extra
> information required by the pixel accessor is the buffered region
> size; it could recover the slices from the input pointer. I like this
> design, but it somewhat involves hijacking the GetBufferPointer()
> method...
> 
> 2. Is it misleading if SliceContiguousPixelContainer actually
> allocates slices in contiguous memory?
> The new container will allow users to import memory from
> non-contiguous slices, but what should it do if allocating the memory
> itself? I guess it is easier to allocate contiguous memory (ie. a
> single buffer in contiguous memory), as long as the user is aware
> there is no guarantee SliceContiguousImage has a contiguous memory
> footprint...
> 
> 3. Not all filters will work with the new SliceContiguousImage. How
> should this be indicated?
> In the case of VectorImage, new vector-based filters had to be added
> because the image type did not work with some of the existing filters.
> This is (thankfully) not the case for SliceContiguousImage -- it
> *should* work with most existing filters/functions/etc. Some examples
> of potential non-interoperable filters include:
> RayCastInterpolateImageFunction, WatershedImageFilter,
> itkVTKImageExport, itkImageFileReader, itkImageFileWriter. Obviously
> it would be good to document and/or inform the user of incorrect
> usage. Perhaps an "\ingroup SliceContiguousImageFilter" tag can be
> added to supported filters? Could concept checking also be used to
> help guide the user?
> 
> That's all for now, although I might have a few more questions when I
> take a look at the neighborhood accessor.
> 
> Thanks again, Dan
> 
> 2008/5/26 Karthik Krishnan <karthik.krishnan at kitware.com>:
> 
>>Dan:
>>
>>On Sun, May 25, 2008 at 2:41 AM, Dan Mueller <dan.muel at gmail.com> wrote:
>>
>>>Image::GetBufferPointer() is used extensively within the toolkit. Take
>>>a look at the constructors for ImageConstIterator,
>>>ConstNeighborhoodIterator, ImageReverseConstIterator,
>>>ImageConstIteratorWithIndex, etc. I have added a list to
>>>   http://www.itk.org/Wiki/Proposals:Slice_contiguous_images
>>>however I realise now that some of these are
>>>ImportImageContainer::GetBufferPointer() (I'll fix this up when time
>>>permits).
>>
>>That should not be a cause of concern
>>  image->GetBufferPointer() is just a pointer to the first pixel in
>>the image. It is the dereferencing (m_BufferPointer + offset) that's
>>of concern.
>>
>>
>>>Unfortunately, while your suggestion of using the PixelAccessor and
>>>PixelAccessorFunctor would have been easy, I think it is too good to
>>>be true.
>>
>>It is true.
>>
>>
>>>The assumptions regarding contiguous memory allocation are
>>>deeply entrenched.
>>
>>These classes were introduced a year or two ago, precisely because we
>>had to support an alternate memory model image for NAMIC.
>>
>>
>>>Take for example the very first step you discussed, the act of passing
>>>the input pixel value from the iterator.Get() function to the
>>>PixelAccessorFunctor:
>>>
>>>>  PixelType iterator.Get()   reads
>>>>   { return pixelAccessorFunctor.Get(*(m_Buffer+m_Offset)); }
>>
>>The variable is passed by reference, however it is later dereferenced
>>by the functor to the corrent pointer. The VectorPixelAccessor reads:
>>
>> inline ExternalType Get( const InternalType & input, const unsigned
>>long offset ) const
>>   {
>>   ExternalType output( (&input)+(offset*m_OffsetMultiplier) ,
>>m_VectorLength );
>>   return output;
>>   }
>>
>>ie the offset is passed in and based on the vector length instead of
>>returning *(m_Buffer+m_Offset) that an itk::Image would do, the
>>itk::VectorImage returns *(m_Buffer + m_Offset * m_VectorLength ) .
>>
>>
>>>This deference *(m_Buffer+m_Offset) will access potentially invalid
>>>memory in the slice-contiguous case (m_Buffer is the result of
>>>m_Image->GetBufferPointer(), the start of the contiguous array, or the
>>>start of the first slice for my case).
>>
>>I realize that it would have been better to pass in the offset and the
>>buffer_start_pointer in, to avoid invalid de-references. (I think
>>passing them by reference avoids the de-reference). So we could change
>>the signature of the the accesssor to
>>
>> inline ExternalType Get( const InternalType & begin, const unsigned
>>long offset ) const
>>
>>and the iterator would read:
>>
>> PixelType Get(void) const
>>   { return m_PixelAccessorFunctor.Get(m_Buffer, m_Offset); }
>>
>>instead of
>>
>> PixelType Get(void) const
>>   { return m_PixelAccessorFunctor.Get(*(m_Buffer+m_Offset)); }
>>
>>
>>
>>>There are other places this assumption is also made: for example
>>>ImageConstIteratorWithIndex::SetIndex() uses the buffer pointer to
>>>compute the index.
>>
>>Again the starting pointer should not matter. All attempts to
>>dereference the pointers, via Set or Get go through the accessors.
>>
>>
>>>Have I misinterpreted your meaning? I think either m_Buffer and/or
>>>m_Offset would need to altered from their original meaning to work for
>>>slice-contiguous memory...
>>
>>No. I don't think so. In your case,
>>
>>m_Buffer would be any arbitrary value say 0.
>>m_Offset would be the Nth pixel into the image.
>>
>>Any attempt to access the Nth pixel would go through the accessor,
>>which would have start pointers to all the slices in your image and
>>would read something like:
>>
>> inline ExternalType itk::SlicePixelAccessor::Get( const InternalType
>>& begin, const unsigned long offset ) const
>>   {
>>   const unsigned long slice = offset/(x_size*y_size);
>>   const unsigned long offset_into_slice = offset % (x_size*y_size);
>>   return *(m_SlicePointer[slice] + offset_into_slice);
>>   }
>>
>>
>>--
>>Hope this helps.
>>
>>Thanks
>>--
>>Karthik Krishnan
>>R&D Engineer,
>>Kitware Inc.
>>
> 
> _______________________________________________
> 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
>