[Insight-developers] RecursiveMultiResolutionPyramidImageFilter -- questions about fixing bug 8482

[kent williams]

OK, I'm going to do so and check that in, along with my modifications to
RecursiveMultiResolutionPyramidImageFilter.


On 3/25/09 1:50 PM, "Bill Lorensen" <bill.lorensen at gmail.com> wrote:

> We should change it back to what it was; SetUseImageSpacing(false). I
> made the change and can't recall why. The comments to the commit do
> not specify a reason.
> 
> On Wed, Mar 25, 2009 at 11:31 AM, Bradley Lowekamp
> <blowekamp at mail.nih.gov> wrote:
>> 
>> This was changed was for no apparent reason in ITK 3.12.
>> http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Algorithms/itkMultiResolutionPyra
>> midImageFilter.txx?root=Insight&r1=1.26&r2=1.27&sortby=date
>> The previous behavior (which is how they had been working since 2004) is the
>> correct behavior!
>> For me this is VERY significant I have already waisted a significant amount
>> of time trying to getting better results in my registration.
>> I would even suggest that this should be a patch to 3.12!
>> Brad :(
>> 
>> 
>> On Mar 25, 2009, at 2:19 PM, kent williams wrote:
>> 
>> So you¹re saying smoother->SetUseImageSpacing(true) may be causing your
>> problem?
>> 
>> At a minimum, in order to A) support what many people consider Œcorrect¹
>> behavior and B) maintain backwards compatibility, it would be good to add a
>> SetUseImageSpacingForSmoothing method.  That way, if you know you don¹t want
>> Image Spacing to affect smoothing, you can turn it off. As it stands now
>> you¹re stuck with it.
>> 
>> 
>> 
>> On 3/25/09 1:14 PM, "Bradley Lowekamp" <blowekamp at mail.nih.gov> wrote:
>> 
>> Hrmm this is interesting!!!
>> 
>> I am currently running slice to slice registration on  scanning electron
>> microscopy data. This data is very noise compared to most medical imaging.
>> This Image spacing parameter could significantly change registration! it has
>> been running poorly recently and this may in fact be the cause. I will have
>> to run things to determine if this is the cause... But a change like this
>> would be very significant to my code :(
>> 
>> Also I don't see any way that the user could set a parameter to change this
>> too!  If you had an image will large spacing it would in effect NOT apply a
>> smoothing filter.
>> 
>> If I am understanding this correctly I would think it is a bug!
>> 
>> Brad
>> 
>> On Mar 25, 2009, at 1:31 PM, kent williams wrote:
>> 
>> I don't have an opinion about whether ImageSpacing should be used in these
>> filters.  I didn't even know what a MultiResolutionPyramidImageFilter was
>> used for until this morning, when I read the comments in the header.
>> 
>> So to summarize, Tom thinks neither filter should use ImageSpacing for
>> smoothing. Currently the RecursiveMulti... filter does not, but the
>> MultiResolution... filter does.
>> 
>> So the choices are
>> 
>> 1. Maintain the status quo (which is inconsistent).
>> 2. Make the two consistently use image spacing (which Tom thinks is wrong in
>> both cases).
>> 3. Make the two both NOT use image spacing for smoothing (which 'breaks
>> backward compatibility' w.r.t. the MultiRes...Filter).
>> 
>> I think in the long run choice 3 is the best, if in fact using image spacing
>> when smoothing is the Wrong Thing To Do.  Failing that, we need to add a
>> flag on the filter so that the behavior can be turned off for users like
>> Tom, who don't want it.
>> 
>> On 3/25/09 12:00 PM, "Tom Vercauteren" <tom.vercauteren at gmail.com> wrote:
>> 
>> Hi Kent,
>> 
>> In making RecursiveMultiResolutionPyramidFilter consisitent with
>> MultiResolutionPyramidFilter, I made the smoother use ImageSpacing.  This is
>> consistent; is it desirable?
>> 
>> Both multi-resolution pyramids define their sigmas in terms of pixel
>> sizes, not physical space. Therefore, they should use a smoother with
>> UseImageSpacingOff. This is what
>> http://public.kitware.com/Bug/view.php?id=503 says.
>> 
>> I really don't understand why ITK 3.12 introduced UseImageSpacingOn
>> instead of UseImageSpacingOff in the smoother of
>> MultiResolutionPyramidFilter. This makes no sense to me and broke
>> bachwards compatibility.
>> 
>> A patch should really not introduce this bug also in
>> RecursiveMultiResolutionPyramidFilter. I would definitely vote for
>> patching MultiResolutionPyramidFilter w.r.t. smoother spacing. This
>> will make both pyramids a little more consistent and would fix the bug
>> introduced in 3.12. It would not be strictly backwards compatible with
>> 3.12 but should be with 3.10 and earlier.
>> 
>> Let me know if I am missing something.
>> 
>> Regards,
>> Tom
>> 
>> 
>> 
>> 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 <http://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
>> 
>> 
>> 
>> 
>> 
>> ========================================================
>> 
>> Bradley Lowekamp
>> 
>> Lockheed Martin Contractor for
>> 
>> Office of High Performance Computing and Communications
>> 
>> National Library of Medicine
>> 
>> blowekamp at mail.nih.gov
>> 
>>