[Insight-developers] Performance of itkShrinkImageFilter

Fri Apr 3 12:36:30 EDT 2009

Brad,

I assume these tests were run with a Release build?

Bill

On Fri, Apr 3, 2009 at 9:11 AM, Bradley Lowekamp <blowekamp at mail.nih.gov> wrote:
> I did some performance testing of the critical loop inside the
> ShrinkImageFilter. The two options are:
> loop 1)
>     outputIndex = outIt.GetIndex();
>     inputIndex = outputIndex * factorSize + offset;
>     outIt.Set( inputPtr->GetPixel(inputIndex) );
> or
> loop 2)
>     outputIndex = outIt.GetIndex();
>     outputPtr->TransformIndexToPhysicalPoint(outputIndex, outputPoint);
>     inputPtr->TransformPhysicalPointToIndex(outputPoint, inputIndex);
>     outIt.Set( inputPtr->GetPixel(inputIndex) );
> I utilized the itkShrinkImageStreamingTest to profile the performance on my
> 2.4GHz Duo MacBookPro and   2x3Ghz Quad-core MacPro.  The image size
> was adjusted to 8Kx12K on my macbook 80Kx12K on the workstation for the test
> to take an about 10 seconds. The other parameter to consider in
> multi-threading, but that complicates things, so I set
>  GlobalMaximumNumberOfThreads to 1.
> On the laptop loop 2 ran in 11 seconds and loop 1 ran in 6.
> On the workstation loop 2 ran in 11.5 seconds and loop 1 ran in 9.
> I then noticed that the filter was using an ImageRegionIterator instead of
> ImageRegionIteratorWithIndex. The latter iterator is theoretically an
> optimized version of the prior when the GetIndex methods is used frequently,
> as we are in this case.
> On the workstation WithIndex loop 2 ran in 8.4 and loop 1 ran in 6.75.
>
> As these two loops are provably equivalent based on the assumption of the
> output information set in the generate data method, I don't see a
> significant down side to reverting this loop to how it was implemented
> before along with the change in the iterator too.
> Brad
>
>
>
> On Mar 28, 2009, at 3:33 PM, Bill Lorensen wrote:
>
> The Shrinker changes were in part meant to maintain alignment of the
> centr's of the subsample volume with the original volume.
>
> As for performance, we should benchmark the old and new versions with
> optimized compilations on a couple of platforms.
>
> Bill
>
> On Thu, Mar 26, 2009 at 5:34 PM, Bradley Lowekamp
> <blowekamp at mail.nih.gov> wrote:
>
> I have just been asking for a clarification of what this bug means. Based on
>
> Han's response:
>
> Brad,
>
> The problem is that the origin is not properly computed in the down sampled
>
> images, and the origin affects the location.
>
> Hans
>
> These changes:
>
> http://public.kitware.com/cgi-bin/viewcvs.cgi/Code/Algorithms/itkMultiResolutionPyramidImageFilter.txx?root=Insight&r1=1.25&r2=1.28
>
> http://public.kitware.com/cgi-bin/viewcvs.cgi/Code/Algorithms/itkRecursiveMultiResolutionPyramidImageFilter.txx?root=Insight&r1=1.15&r2=1.16
>
> in fact, these changed don't effect the origin or the location of the image.
>
> So I really don't understand what was trying to be achieved here.
>
> To add more to the confusion the description Luis added for the patch
>
> doesn't seem right either:
>
> http://public.kitware.com/Bug/view.php?id=8795
>
> The change from shrinker to a linear resampler is arbitrary, ill motivated,
>
> and changes the intrinsic behavior of these filter. These changes did not
>
> solve the bug and are quickly creating a nest of confusing mantis entries!
>
> With many people working on individual problems, and no one looking at the
>
> big picture.
>
> Hopefully there is a TCON tomorrow. I think it would be best if this could
>
> be discussed there, to determine what the correct behaviors and and what
>
> should be done.
>
> Brad
>
> On Mar 26, 2009, at 6:02 PM, kent williams wrote:
>
> Brad, the switch to ResampleImageFilter to ShrinkImageFilter occurred n
>
> 12-17-2008, and are part of the 3.12 release.  The small change I checked in
>
> this week — which Luis is back-porting into 3.12 — just turns off
>
> UseImageSpacing, which was the default behavior prior to 12-18-2008.
>
> I changed RecursiveMultiResolutionPyramidImageFilter this week to also use
>
> ResampleImageFilter in preference to ShrinkImageFilter.   This was a change
>
> requested after some discussion, and logged in the Bug Tracker:
>
> http://public.kitware.com/Bug/view.php?id=8482
>
> This change did not break any existing regression tests.  Obviously, it
>
> would be good to have a regression test that failed before this change and
>
> succeeds afterwards.  When Hans gets back in the office next week, I’ll
>
> discuss formulating such a test.
>
> Conversely, if this change introduces some behavior you find undesirable it
>
> would be great if you could come up with a test program that illustrates the
>
> problem.
>
> I don’t have a dog in this fight, I just try to fix the bugs I’m assigned
>
> without breaking anything obvious.  I studied Comp Sci and not Medical Image
>
> Processing background so every time I tackle one of these filters, I don’t
>
> even necessarily know what it does beforehand, or have any idea what would
>
> constitute improved or impaired behavior.
>
> I must ask then why are you trying to fix it if you don't know what is
>
> broken or what is correct?
>
>
> On 3/26/09 4:43 PM, "Hans Johnson" <hans-johnson at uiowa.edu> wrote:
>
> 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
>
> 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
>
>