[Insight-users] itkHistogram with double, comments

[Michal Sofka]

One additional consideration -- it would be good if we don't force the 
histogram's frequency type to be float only. This might cause problems 
with large histograms. For example when the total frequency is more than 
1e+7, than increasing frequency by 1 (in any bin) does not change total 
frequency member variable (because of lack of precision). I've tried this 
case and there are ways around it but it would be nice if it works in 
general.


Michal.



> 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
>
>
>