[Insight-developers] So -- no concensus on smoothing re MultiResolutionPyramidImageFilter?

Thu Apr 9 08:34:06 EDT 2009

Hey Hans,

Not that I like this consensus but I can live with it. I would however
push for using a three state flag within
RecursiveMultiResolutionPyramidImageFilter and not a boolean one:
  - State A: Never smooth "unit shrinkage images" (no matter what the
pyramid schedule is)
  - State B: Always smooth "unit shrinkage images" (no matter what the
pyramid schedule is)
  - State C: Smooth "unit shrinkage images" if and only if the
schedule is not backward compatible

Following this discussion, default would be State C to maintain
complete backward compatibility.

As a side not, I already added some initial comments about the
inconsistent smoothing a few weeks ago:
http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Algorithms/itkRecursiveMultiResolutionPyramidImageFilter.h?root=Insight&r1=1.13&r2=1.14&sortby=date
http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Algorithms/itkMultiResolutionPyramidImageFilter.h?root=Insight&r1=1.20&r2=1.21&sortby=date

Of course, feel free to enhance these notes.

Cheers,
Tom

On Thu, Apr 9, 2009 at 14:21, Hans Johnson <hans-johnson at uiowa.edu> wrote:
> GREAT!  A consensus is being reached.
> Result: Backward compatibility trumps fixing inconsistent behavior!
>
> So the current proposed plan to be able to close the bug is:
> 1) For the MultiResolutionPyramidImageFilter the default is true. A note
> should be added to the documentation that this filter does not (by default)
> conform to the standard definition of an Image Pyramid.  Explicitly state
> that this filter will only give the same results as the
> RecursiveMultiResolutionPyramidImageFilter when the image is NOT "downward
> divisible".
>
> 2) For the RecursiveMultiResolutionPyramidImageFilter the default is false.
> A node should be added to the documentation that states that the filter will
> continue to change behavior depending on the "downward divisibility" of the
> input data with respect to the shrink schedule (the current behavior).
>
> Thanks for you comments and suggestions.
>
> Hans
>
>
> On 4/9/09 6:59 AM, "Bradley Lowekamp" <blowekamp at mail.nih.gov> wrote:
>
>> Again. Please to do not make these changes to in a way that will
>> effect peoples registrations! That is don't change the default
>> behavior from how it has been for year!
>>
>> Just because something is reported in Mantis doesn't mean that it is a
>> bug. Some one needs to intelligently evaluate it and determine if it
>> is really a bug or just one user's expected behavior. I expect that my
>> registration will work the same from 3.10 to 3.11 to 3.12! And it has
>> not been!
>>
>> You are welcome to add a new parameter or option. BUT DO NOT CHANGE
>> THE DEFAULT BEHAVIOR! I am getting tired of having to reverify my
>> registration is working correctly.
>>
>> Brad
>>
>
>
>
> On 4/9/09 7:07 AM, "Stephen Aylward" <stephen.aylward at kitware.com> wrote:
>
>> I could be misunderstanding...but shouldn't the default be "true" to
>> keep backward compatibility?
>>
>> Having an option to offer backward compatibility, but having the
>> default be to not enable backward compatibility seems very odd (ok, it
>> seems simply wrong).
>>
>> Bill?
>>
>> Luis?
>>
>> Stephen
>>
>> On Thu, Apr 9, 2009 at 7:30 AM, Hans Johnson <hans-johnson at uiowa.edu> wrote:
>>> Since this is considered a bug for over a year
>>> http://www.vtk.org/Bug/view.php?id=7002 we should consider the fact that
>>> MultiResolutionPyramidImageFilter smooths for unit shrink factors a bug that
>>> would allow us to fix it (even through numerical sameness/backwards
>>> compatibility will be lost).
>>>
>>> This is not the behavior that I would expect by reading the documentation on
>>> how an Image Pyramid should be constructed (i.e. The top level should be an
>>> exact copy of the reference images).
>>>
>>> I think that this is a bug that also affects the
>>> RecursiveMultiResolutionPyramidImageFilter because it would produce
>>> different results for the unit shrink factor image if I change the shrink
>>> schedule from being downward divisible to being non-downward divisible.
>>> This is particularly nasty because given the same "shrink schedule
>>> [8,4,2,1]" the unit shrink result will be different depending on the size of
>>> the input image.
>>>
>>> ===============
>>> If there are no objects today, then the solution that will be implemented
>>> will be:
>>>
>>> 1) Add a test that fails when the unit shrink image is not the same as the
>>> reference image (and the m_SmoothIfUnitShrinkFactors =false).
>>> 2) Add the m_SmoothIfUnitShrinkFactors conditional flag to allow explicit
>>> smoothing of the unit shrink factors.
>>> 3) Set the default to "false" in all cases.
>>> ===============
>>>
>>>
>>> Regards,
>>> Hans
>>>
>>>
>>> On 4/9/09 1:56 AM, "Tom Vercauteren" <tom.vercauteren at gmail.com> wrote:
>>>
>>>> Hi Kent,
>>>>
>>>> Even if there is no consensus about what the default should be, it
>>>> could be add a flag that lets the user decide (as Stephen suggested).
>>>>
>>>> Maybe something like:
>>>>   m_SmoothIfUnitShrinkFactors
>>>>
>>>> For backwards compatibility, the default for this flag would be:
>>>>   - true for MultiResolutionPyramidImageFilter
>>>>   - false for RecursiveMultiResolutionPyramidImageFilter
>>>>
>>>> Now, I also would like to propose a NON-backward compatible change to
>>>> fix something that might be very confusing to the user.
>>>>
>>>> Since RecursiveMultiResolutionPyramidImageFilter wil call
>>>> MultiResolutionPyramidImageFilter if the schedule is not downward
>>>> divisible, I suggest that RecursiveMultiResolutionPyramidImageFilter
>>>> sets
>>>>
>>>> internalNonRecursivePyramid->SetSmoothIfUnitShrinkFactors(this->m_SmoothIfUn
>>>> it
>>>> ShrinkFactors)
>>>> when the schedule is not downward divisible.
>>>>
>>>> With this small non-backward compatible change, it would at least make
>>>> things a bit clearer.
>>>>
>>>> Otherwise we might need a three state flag for
>>>> RecursiveMultiResolutionPyramidImageFilter:
>>>>   - AlwaysSmoothIfUnitShrinkFactors
>>>>   - NeverSmoothIfUnitShrinkFactors
>>>>   - SmoothIfUnitShrinkFactorsAndScheduleDownardDivisible (this being
>>>> the default for true backwards compatibility)
>>>>
>>>> Thoughts anyone?
>>>>
>>>> Tom
>>>>
>>>> On Wed, Apr 8, 2009 at 23:27, kent williams <norman-k-williams at uiowa.edu>
>>>> wrote:
>>>>> It seems Hans assigned me yesterday to fix this 'bug':
>>>>>
>>>>> http://www.vtk.org/Bug/view.php?id=7002
>>>>>
>>>>> As is noted there, there still doesn't appear to be a consensus on how to
>>>>> handle this problem.  So I've taken ownership of a bug with no clear way to
>>>>> fix it.
>>>>>
>>>>> If the issue is truly tabled, does it need to hang around as an open bug?
>>>>>
>>>>>
>>>>>
>>>>> 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
>>>>>
>>>> _______________________________________________
>>>> 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
>>>
>>> _______________________________________________
>>> 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
>>>
>>
>>
>
>