[Insight-users] itkHistogram with double, comments

[Miller, James V (Research)]

Stephen, 

I think this is similar to other arguments.  You are basically
saying that GetFrequency/GetTotalFrequency always return double
regardless of how the frequency is stored in the FrequencyContainer.


Jim


-----Original Message-----
From: Stephen R. Aylward [mailto:aylward at unc.edu]
Sent: Thursday, May 19, 2005 4:35 PM
To: Miller, James V (Research)
Cc: Michal Sofka; insight-users at itk.org
Subject: Re: [Insight-users] itkHistogram with double, comments



GetFrequency and GetTotalFrequency are pure virtual functions in the 
class Sample.

Why not have those functions return double, do away with the concept of 
FrequencyType in the Sample class, and let the subclasses instantiate 
FrequencyType as they see fit?

Stephen


Miller, James V (Research) wrote:
> Michal, 
> 
> Thanks for pointing this out.
> 
> For #1, I am not sure how we want to fix it.
> 
> a) Make Sample templated over frequency type.  Then Histogram can define
> its superclass based by extracting the frequency type from the FrequencyContainer
> that the histogram is templated over. I don't really like this approach because
> I hate having to add a template parameter to such a base class.  But the 
> template parameter in the base class could default to float, minimizing the
> impact to backward compatibility.
> 
> b) Histogram could define its FrequencyType from its superclass and accept any
> change in precision when communicating with the FrequencyContainer. I don't like
> this approach because it potentially hides a loss in precision and requires
> casting at every GetFrequency() call.
> 
> c) Change the FrequencyContainers to NOT be templated over frequency type
> and just define FrequencyType to ALWAYS be float.  I don't like this 
> because existing code will no longer compile.
> 
> d) Use ConceptChecking so that Histogram can only use FrequencyContainers that
> are use floats as the FrequencyType.  All this buys us from the current situation
> is a potentially clearer compiler error.
> 
> As you can tell, I am not particularly happy with any of these solutions.  Perhaps
> another developer may have another idea.
> 
> I fixed the documentation errors you indicated.
> 
> Jim
> 
> 
> 
> 
> -----Original Message-----
> From: insight-users-bounces+millerjv=crd.ge.com at itk.org
> [mailto:insight-users-bounces+millerjv=crd.ge.com at itk.org]On Behalf Of
> Michal Sofka
> Sent: Thursday, May 19, 2005 1:54 PM
> To: insight-users at itk.org
> Subject: [Insight-users] itkHistogram with double, comments
> 
> 
> 
> Hi itk users (and developers),
> 
> I've been using itkHistogram and found several inconsistencies:
> 
> 1. It is derived from itkSample, that has
>   typedef float FrequencyType ;
> but the Histogram itself is templated over FrequencyType (through
> TFrequencyContainer). As a result, when instantiating with a different type
> for TFrequencyContainer than float, there are conflicts in GetFrequency and
> GetTotalFrequency return types (see error messages below). (I'm using VS7.0,
> Win2000.)
> 
> 2. Comments for IncreaseFrequency functions (all prototypes) is not right
> /** Method to increase the frequency by one.  This function is convenient
>     to create a histogram. It returns false if the bin is out of bounds. */
> It increases the frequency by 'value'.
> 
> 3. Comment before the class (in the header file, line 46) is not right:
>  * After this, users want to set range of each bin using
>  * SetBinMin(dimension, n) and SetBinMax(dimension, n) methods.
> The prototype of these functions is different.
> 
> 
> Thanks.
> 
> Michal.
> 
> 
> 
> -----------------------------------
> Michal Sofka, PhD student
> Department of Computer Science, RPI
> sofka at cs.rpi.edu
> 
> 
> 
> 
> Issue 1 errors:
> 
> itkHistogram.h(231) : error C2555:
> 'itk::Statistics::Histogram<TMeasurement,VMeasurementVectorSize,TFrequencyCo
> ntainer>::GetFrequency': overriding virtual function return type differs and
> is not covariant from
> 'itk::Statistics::Sample<TMeasurementVector>::GetFrequency'
>         with
>         [
>             TMeasurement=double,
>             VMeasurementVectorSize=6,
> 
> TFrequencyContainer=itk::Statistics::DenseFrequencyContainer<double>
>         ]
>         and
>         [
>             TMeasurementVector=itk::FixedArray<double,6>
>         ]
>         itkSample.h(112) : see declaration of
> 'itk::Statistics::Sample<TMeasurementVector>::GetFrequency'
>         with
>         [
>             TMeasurementVector=itk::FixedArray<double,6>
>         ]
> itkHistogram.h(282) : error C2555:
> 'itk::Statistics::Histogram<TMeasurement,VMeasurementVectorSize,TFrequencyCo
> ntainer>::GetTotalFrequency': overriding virtual function return type
> differs and is not covariant from
> 'itk::Statistics::Sample<TMeasurementVector>::GetTotalFrequency'
>         with
>         [
>             TMeasurement=double,
>             VMeasurementVectorSize=6,
> 
> TFrequencyContainer=itk::Statistics::DenseFrequencyContainer<double>
>         ]
>         and
>         [
>             TMeasurementVector=itk::FixedArray<double,6>
>         ]
>         itkSample.h(115) : see declaration of
> 'itk::Statistics::Sample<TMeasurementVector>::GetTotalFrequency'
>         with
>         [
>             TMeasurementVector=itk::FixedArray<double,6>
>         ]
> 
> _______________________________________________
> Insight-users mailing list
> Insight-users at itk.org
> http://www.itk.org/mailman/listinfo/insight-users
> _______________________________________________
> Insight-users mailing list
> Insight-users at itk.org
> http://www.itk.org/mailman/listinfo/insight-users

-- 
===========================================================
Dr. Stephen R. Aylward
Associate Professor of Radiology
Adjunct Associate Professor of Computer Science and Surgery
http://caddlab.rad.unc.edu
aylward at unc.edu
(919) 966-9695