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

Thu Apr 9 09:34:58 EDT 2009

I agree.

On Apr 9, 2009, at 9:28 AM, Luis Ibanez wrote:

>
>
>           This filter is reaching the point
>           where it is not recoverable.
>
>
> This discussion start looking like a symposium on how
> to fix a duck-tape patch with another duck-tape patch.
>
>
> I vote for :
>
>
>     a) Leaving it in a state that is closer to what we
>        could call backward compatible.
>
>     b) Then mark it as deprecated,
>        and discourage anybody from using it again.
>
>     c) Write a new filter(s), with a new name(s).
>        From a fresh start based on the requirements
>        that we know today.
>
>
> The requirements could be:
>
>     1) Performance when no smoothing is needed.
>
>     2) Keeping the center of mass of the images at different levels.
>
>     3) ...more.. ?
>
>
> If the requirements turn out to be conflictive, then I would vote
> for writing separate variations of the filter, and make each one
> of them fine tunned for its correct use.
>
> Saturating a class with booleans and enums is a symptom that we
> are failing to design a hierarchy of classes where the various
> behaviors should be implemented in separate derived classes.
>
> Using the "boolean" and "enum" approach, one would have to wonder
> why is that we don't have a single monstrous itkOptimizer class,
> and through dark-magic combinations of boolean settings we make it
> behave like a : GradientDescent, or Powell, or Evolutionary....,
> or why don't we have a single itkImageMetric class, and through
> enums settings we make it behave as Mutual Information, or  
> MeanSquares,
> or NormalizedCorrelations...
>
> There is no reason why we have to stick to a single
> "MultiResolutionPyramidImageFilter" when we could have a clean
> framework/hierarchy with multiple classes, where each one:
>
>
>               "do one thing, do it well"
>
>
>
>    Luis
>
> --
>
>
> PS.
>
>     High density of "if" statements (and its cousin, the "switch")
>     is an indicator of poor software design, and/or poor software
>     maintenance practices. We can do much better than that...
>
>
> ----------------------
> Stephen Aylward 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_SmoothIfUnit
>>>> 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
>>>
>>
>>
>>
>>
> _______________________________________________
> 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20090409/3a2a7385/attachment-0001.htm>