[Insight-developers] The {Binary|Ternary}FunctorImageFilter problem.

Jim Miller millerjv at gmail.com
Sun Jun 5 13:34:58 EDT 2011 

Brad 

My concern was really the extra memory that would be used in a resample balanced against user expectations. For instance, the expectation would be that adding two images is such a trivial operation that the amount of memory used would be limited to at most one extra memory buffer. If run inplace, the expectation would a zero memory overhead. 

Now, if not run inplace, then the resample and the output buffer could be shared.....

The rest of your suggestions, update progress less often, thread pools, and line iteration all sound good. Thread pooling is probably the hardest to do cross platform. We should devise a nice test so we can measure performance change. 

For progress, isn't the actual event only being trigger a small number if times per filter? Are we using a lot of overhead on the decrement and test condition on the counter? Or are we over triggering the events? Can't imagine a need of triggering the events more than 100 times a filter, if that. 

Did we ever find a good thread pool library for Linux and windows? You had used the native MacOS one. 

Jim

On Jun 5, 2011, at 1:01 PM, Bradley Lowekamp <blowekamp at mail.nih.gov> wrote:

> Jim,
> 
> Underlying  part of what you are saying is that trivial filters are expensive to operate and running multiple filters should be avoided.
> 
> I have the opposite view on how this should be fixed. I think these trivial filters should be made faster, so that there is less overhead and trivial to execute.
> 
> There are 3 optimization that should be performed on these filters. I feel confident that we should be able to get a 4x performance increase for these filters. The following optimizations I have been pondering doing:
> 
> 1) The progress should only be incremented on a per line basis. As demonstrated with image copy, this can take up to 50% of the execution time of trivial operations.
> 
> 2) Introducing thread pools on systems, which have a thread pool library should reduce the over head of calling filters for systems with large number processors.
> 
> 3) Change to a line iterator which translates to a trivial iterator ( a pointer ) on a per line basis. This will enable intelligent compilers to optimize the code to SIMD/SSE instructions.
> 
> 
> While all executions of images on all systems may not show a significant performance improvement, all three of these should help in a variety of frequently occurring situations.
> 
> This is one reason why I would like to keep simple filters simple.
> 
> Do we have any performance number for the cost of running the resample then the add filters, vs just the resample and add filter?
> 
> Brad
> 
> 
> On Jun 5, 2011, at 11:54 AM, Jim Miller wrote:
> 
>> I think for the simple filters like the functor filters it is best to avoid inserting a resample filter but rather resample as you go pixel by pixel. This can either be done by iterating one input and calling index to physical then physical to index and interpolating the other inputs or by caching the composition of the two transforms like the resample filter does. 
>> 
>> These filters could operate in two modes. If the physical and index coordinate frames match up they run as is. Otherwise they switch to mapping indices to physical space. 
>> 
>> But we need a couple of architecture things. One a change in the pipeline to check on the compatibility of the inputs physical and index space. And a second to indicate how my it cares. I could see three ways to write a filter
>> 
>> 1. Like today. The physical spaces and index spaces need to match. 
>> 
>> 2. The physical spaces need to match but the index spaces do not. I.e. The images span perhaps overlapping physical spaces but the ordering of the pixels is different. 
>> 
>> 3. No matching of spaces required. 
>> 
>> I am wondering whether all the GenerateInputRequestedRegion methods should be rooted in physical space (and figure out what index space is needed for a given physical space request). Sound pretty ugly to retrofit. 
>> 
>> 
>> On Jun 3, 2011, at 11:43 AM, Cory Quammen <cquammen at cs.unc.edu> wrote:
>> 
>>> You may not need a separate hierarchy of classes for performing
>>> binary/ternary/nary filters. I can imagine a templated class
>>> 
>>> template< class TBinaryFunctorImageFilter, class TInterpolator >
>>> class PhysicalSpaceBinaryFunctorImageFilter : public SomeAppropriateSuperclass
>>> 
>>> that has an internal mini-pipeline which resamples the input images to
>>> the grid with the desired output spacing, origin, orientation, etc.,
>>> and runs the binary image filter on the resampled images. It would be
>>> rough on memory, but you could make some optimizations such as only
>>> resampling one of the images if you want the output image to have the
>>> same physical attributes as the other image. I should point out that
>>> some users may want to be able to specify completely different
>>> physical spacing, origin, and orientation for the output of the
>>> PhysicalSpaceBinaryFunctorImageFilter.
>>> 
>>> For adding images in physical space, you could define
>>> 
>>> typedef PhysicalSpaceBinaryFunctorImageFilter< AddImageFilter,
>>> LinearInterpolatorType > PhysicalSpaceAddImageFilter;
>>> 
>>> However, if you do want to add a separate hierarchy, such a class
>>> would make defining it very easy and minimize code duplication.
>>> 
>>> Cory
>>> 
>>> On Thu, Jun 2, 2011 at 11:37 AM, Luis Ibanez <luis.ibanez at kitware.com> wrote:
>>>> Hi Kent,
>>>> 
>>>> On Thu, Jun 2, 2011 at 10:49 AM, Williams, Norman K
>>>> <norman-k-williams at uiowa.edu> wrote:
>>>>> I feel like I saw I way to implement what Hans was asking for, and
>>>>> foolishly I thought that adding the functionality near the base of the
>>>>> inheritance hierarchy was actually a win -- it added new functionality to
>>>>> a whole family of classes, without changing their behavior for existing
>>>>> cases.
>>>>> 
>>>>> It has turned into a big mess. Partly that was my fault, because I was
>>>>> doing something that was a bit tricky internally.  I also pushed it up to
>>>>> Master based on limited feedback and without much testing on compilers
>>>>> aside from gcc.
>>>>> 
>>>> 
>>>> 
>>>> I also have to share the blame here,
>>>> since I approved that original patch
>>>> and missed to see the implications
>>>> that Brad L. pointed out yesterday.
>>>> 
>>>> 
>>>>> So I guess what Luis is proposing is this:
>>>>> 
>>>>> 1. Back out the changes I made. That means reversing this patch:
>>>>> http://review.source.kitware.com/#change,1725
>>>>> 
>>>> 
>>>> Yes,
>>>> This is what is being proposed in this patch:
>>>> http://review.source.kitware.com/#change,1777
>>>> 
>>>> 
>>>>> 2. Add space checking to the existing functions, at the
>>>>> {Binary|Ternary}FunctorImageFilter level. Throw exception
>>>>> if spaces don't match to a tolerance.
>>>>> 
>>>> 
>>>> Yes,
>>>> 
>>>> and... also
>>>> 
>>>> a) Adding unit tests for these classes.
>>>> 
>>>> b) Moving the UnaryFunctorImageFilter from ITK-Common
>>>>   to ITK-ImageFilterBase so, they are all together in the
>>>>   same directory (Unary,Binary,Ternary).
>>>> 
>>>> 
>>>>> 3. Start a new, parallel tree of functor filters.  Probably needs to be
>>>>> in its own, new module?  What is it called?
>>>>> 
>>>> 
>>>> 
>>>> Not sure about whether they need a new module,
>>>> but certainly a new hierarchy is desirable.
>>>> 
>>>> 
>>>>> Forgive me, Luis if I'm mis-representing what you've said.
>>>>> 
>>>> 
>>>> You got it all right.
>>>> 
>>>>> I also think Hans is right, in that the existing system behaves in
>>>>> surprising, incorrect ways for medical imaging. Here at Iowa we have a lot
>>>>> of experience with using and teaching ITK at the undergraduate, graduate
>>>>> and professional levels. Part of the steepness of the learning curve is
>>>>> avoiding pitfalls that arise from the pitfalls associated with physical vs
>>>>> image space.
>>>>> 
>>>>> But I want to be done with this controversy.  I can implement whatever
>>>>> consensus solution is decided on.
>>>>> 
>>>>> 
>>>>> 
>>>>> ________________________________
>>>>> Notice: This UI Health Care e-mail (including attachments) is covered by the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is confidential and may be legally privileged.  If you are not the intended recipient, you are hereby notified that any retention, dissemination, distribution, or copying of this communication is strictly prohibited.  Please reply to the sender that you have received the message in error, then delete it.  Thank you.
>>>>> ________________________________
>>>>> _______________________________________________
>>>>> Powered by www.kitware.com
>>>>> 
>>>>> Visit other Kitware open-source projects at
>>>>> http://www.kitware.com/opensource/opensource.html
>>>>> 
>>>>> Kitware offers ITK Training Courses, for more information visit:
>>>>> http://kitware.com/products/protraining.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
>>>>> 
>>>> _______________________________________________
>>>> Powered by www.kitware.com
>>>> 
>>>> Visit other Kitware open-source projects at
>>>> http://www.kitware.com/opensource/opensource.html
>>>> 
>>>> Kitware offers ITK Training Courses, for more information visit:
>>>> http://kitware.com/products/protraining.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
>>>> 
>>> 
>>> 
>>> 
>>> -- 
>>> Cory Quammen
>>> Computer Integrated Systems for Microscopy and Manipulation (CISMM)
>>> Department of Computer Science
>>> University of North Carolina at Chapel Hill
>>> http://www.cs.unc.edu/~cquammen
>>> _______________________________________________
>>> Powered by www.kitware.com
>>> 
>>> Visit other Kitware open-source projects at
>>> http://www.kitware.com/opensource/opensource.html
>>> 
>>> Kitware offers ITK Training Courses, for more information visit:
>>> http://kitware.com/products/protraining.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
>> _______________________________________________
>> Powered by www.kitware.com
>> 
>> Visit other Kitware open-source projects at
>> http://www.kitware.com/opensource/opensource.html
>> 
>> Kitware offers ITK Training Courses, for more information visit:
>> http://kitware.com/products/protraining.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
>

More information about the Insight-developers mailing list