[Insight-developers] NormalizedCorrelationImageFilter::SetTemplate parameter type

[Bradley Lowekamp]

Hello David,

Clarifying documentation is certainly a good thing.
Hello David,

The NeighthoodOperatorImageFitler is widely used as a base class, changing this type would likely cause a ripple effect across a number of ITK filters and users derived classes. From the the super class' perspective I think the naming convention makes a lot of since, then from the NormalizedCorrelation Filter

The NormalizedCorrelationImageFilter in particular is not done in frequency space so the cost of correlating images of the same size would be quite extreme. It's more geared to template matching. I imagine that this filter was written with the idea of creating a small template to search for, and having the template and the output both be of real type.

I however do agree that the name for the operator value type is poor or even misleading. I however don't know if changing the default template parameter will help new users to the class as it may hurt existing users.....

I think the best thing to do is to clarify the doxygen documentation. You already have the excellent wiki example to demonstrate the usage, so that should go along way with helping people use the class.

So what I am I saying should be done here... It's not clear to me. It looks like is should be a small patch... perhaps if it's put up for review in gerrit, the +1, and -1 will figure out what's best....

Brad


On Jun 26, 2012, at 12:20 PM, David Doria wrote:

> On Tue, Jun 26, 2012 at 9:42 AM, Bradley Lowekamp <blowekamp at mail.nih.gov> wrote:
> David,
> 
> This certainly sounds like a bug.
> 
> Do you have a patch?
> 
> If the patch is not invasive and gets approved today, it may... just may be able to make it into the RC4.
> 
> Brad
> 
> Hi Brad,
> 
> I think there are actually three separate problems/questions here - 
> 
> 1)
> I realized that I can specify a 4th template parameter (TOperatorValueType):
> 
>   typedef itk::NormalizedCorrelationImageFilter<FloatVectorImageType, MaskType,
>                                                 FloatImageType, FloatVectorType> CorrelationFilterType;
>   CorrelationFilterType::Pointer correlationFilter = CorrelationFilterType::New();
> 
> However, when I do this, I get a concept check error that ‘itk::CovariantVector<float, 3u> can't be casted to type ‘float’. I'm assuming this means that this filter can only operate on scalar images? I guess now that I think about it, the standard correlation computation doesn't seem to naturally extend to vector-valued pixels... Perhaps there should at least be a mention of this in the documentation, or better, a concept check on TInputImage.
> 
> 2)
> Though I don't think I'll be able to use the filter as I was trying to in (1), I still think the variable names and default template parameters are confusing, even for correct usage.
> 
> In the parent class:
> 
> template< class TInputImage, class TOutputImage, class TOperatorValueType = typename TOutputImage::PixelType >
> class ITK_EXPORT NeighborhoodOperatorImageFilter:
> 
> we see that TOperatorValueType defaults to the output image type. Wouldn't it make more sense to default to the input image type, since you'd usually be correlating two images of the same type? 
> 
> 3)
> With or without the default in (2), the class then goes on to define:
> 
>   typedef Neighborhood< OperatorValueType,
>                         itkGetStaticConstMacro(ImageDimension) > OutputNeighborhoodType;
> 
> should this be renamed OperatorNeighborhoodType - as I'm not sure what it has to do with 'Output'? That would then trickle down to clarify the confusion with the type of the SetTemplate() parameter.
> 
> If you agree, I can make a patch for 2 and 3, but 1 probably just needs a comment or a new concept check rather than a patch.
> 
> David

========================================================
Bradley Lowekamp  
Medical Science and Computing 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/pipermail/insight-developers/attachments/20120627/fa86566c/attachment.htm>