[Insight-developers] Re: itkSubsample issues

[Karthik Krishnan]

Hi Brad,

IMHO, as you mentioned there is a BCP issue to address here. (While 
there may be no objection here, they'll come in the form of mails in the 
users list later.. we've seen this once before :) ).

For instance if somebody uses a subsample and then sorts it, for 
querying the quantile, and then queries samples as
  subsample->GetMeasurementVector(1),
  subsample->GetMeasurementVector(2)
   .....
will have the unsorted sample instances and with the proposed change 
will have the sorted sample instances. This may not be an uncommon use case.

And while your proposed change is the reasonable behaviour, it will 
break code. The scary thing is that its not something that will generate 
a compiler error, (which I personally think is ok, despite the BCP). It 
quietly changes the runtime behaviour.

Can you not make do with the subsample's iterator API ? or the 
GetMeasurementVectorByIndex method, (which I admit forces you to be 
aware of the fact that its a subsample).

my 2 cents.
-karthik

Brad Davis wrote:

> By the way, I did run ctest (cvs version of Insight from yesterday)
> and all the tests passed with this change in place (linux x86_64, gcc
> 4.0.4).

>
> On 8/15/06, Brad Davis <brad.davis at kitware.com> wrote:
>
>> I propose changing the implementation of GetMeasurementVector in the
>> Subsample class from
>>
>> const MeasurementVectorType & GetMeasurementVector(const
>> InstanceIdentifier &id) const
>> { return m_Sample->GetMeasurementVector(id) ; }
>>
>> to
>>
>> const MeasurementVectorType & GetMeasurementVector(const
>> InstanceIdentifier &id) const
>> { return m_Sample->GetMeasurementVector(m_IdHolder[id]) ; }
>>
>> First, the current implementation allows access to measurement vectors
>> that are not in the subsample---GetMeasurementVector gets vectors from
>> the sample, not the subsample.  Second, while the
>> GetMeasurementVectorByIndex() method can be used to return the
>> measurement vectors that are actually in the sample, the change in
>> interface means that it is difficult to write general code that will
>> work for a sample or a subsample.  Also, GetMeasurementVectorByIndex
>> returs by value rather than by reference.  Iterators can be used but
>> using both iterators and GetMeasurementVectorByIndex() it is
>> impossible to have a working subsample of a subsample---the
>> implementation only allows for one level of indirection.  Again this
>> means that you have to know a priori if you are working with a sample
>> or subsample.
>>
>> The change that I propose allows Samples and Subsamples to be
>> addressed by a common interface, and allows the programmer to create a
>> Subsample of either a Sample or a Subsample.  This would increase the
>> flexibility of the classes and allow for more general code.
>>
>> The drawback of the change is that if someone is already using the
>> GetMeasurementVector() method of the Subsample class to directly (and
>> incorrectly, I argue) access vectors from the Sample, then the change
>> is not backward compatible.  However, access to the Sample can also be
>> gained by Subsample's GetSample method.
>>
>> The other mechanism for fixing this problem is to add a
>> GetMeasurementVectorByIndex() method to the Sample class, and change
>> the method to return by reference.  This does not require changing the
>> GetMeasurementVector method but it makes the classes more confusing.
>>
>> Brad
>>
> _______________________________________________
> Insight-developers mailing list
> Insight-developers at itk.org
> http://www.itk.org/mailman/listinfo/insight-developers
>