[Insight-users] itkHistogram with double, comments
Stephen R. Aylward
aylward at unc.edu
Thu May 19 18:17:24 EDT 2005
Good point - I totally agree that it is not ideal. But perhaps this is
a bit the lessor of the evils? But it is probably more like a belief
than a fact...Removing the typedef as an additional step somewhat
removes the problems regarding the typedef without requiring additional
templates that are not backward compatible for any code (albeit my
solution is still not 100% backward compatible, but hopefully won't
affect too many people) and without dictating the storage type which has
memory implications. The typedef doesn't seem to be used in the
baseclass beyond this return type.
s
Miller, James V (Research) wrote:
> 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
More information about the Insight-users
mailing list