[Insight-developers] Casting constructors for DTI and Sym Mat pixeltypes
Luke Bloy
luke.bloy at gmail.com
Tue Mar 16 16:00:47 EDT 2010
Luis,
The build was green, So I have just committed the patch. I'll keep my
eye on the continuous builds and the nightlies in the morning.
Thanks for the help.
-Luke
Luis Ibanez wrote:
> Hi Luke,
>
> Thanks a lot for implementing the changes
> and preparing the patch.
>
> I tried to run it on my side,
> but the following file is missing:
>
> itkNumericTraitsDiffusionTensor3DPixel.cxx
>
>
> Could you please send this file ?
>
> ---
>
>
> The additional tests look reasonable.
>
>
> A good reference to have is to monitor the code coverage
> of the classes today, and verify that they have the same
> (or larger code) coverage tomorrow.
>
> To be more specific, we have today:
>
> itkSymmetricSecondRankTensor.txx : 98.80%
> http://www.cdash.org/CDash/viewCoverageFile.php?buildid=563343&fileid=10858202
>
> itkSymmetricSecondRankTensor.h : 100%
> http://www.cdash.org/CDash/viewCoverageFile.php?buildid=563343&fileid=10858201
>
> itkDiffusionTensor3D: 98.11%
> http://www.cdash.org/CDash/viewCoverageFile.php?buildid=563343&fileid=10857778
>
>
> ----
>
> If your Experimental build turns out to be green,
> please go ahead and commit the patch.
>
> Then,
> we should keep an eye on the continuous
> builds for the rest of the day.
>
>
> Thanks
>
>
> Luis
>
>
> --------------------------------------------------------
> On Tue, Mar 16, 2010 at 1:15 PM, Luke Bloy <luke.bloy at gmail.com> wrote:
>
>> Hi Luis,
>>
>> I'm attaching a patch that contains the the constructors as well as the
>> NumericTraits for DiffusionTensor3D pixeltypes.
>>
>> Please let me know if the added tests are sufficient.
>>
>> I'm running an Experimental build now If it is green should i commit the the
>> changes?
>>
>> -Luke
>>
>> Luis Ibanez wrote:
>>
>>> Hi Karthik,
>>>
>>> Thanks for pointing this out.
>>>
>>> You are right,
>>> the FixedArray already provide a templated implementation
>>> of the operator=() as well as the casting constructor.
>>>
>>>
>>>
>>> Luke:
>>>
>>> Would you like to give it a shot at implementing these
>>> methods in the itkSymmetricSecondRankTensor ?
>>>
>>> You may want to follow the example of
>>>
>>> itkFixedArray.h lines:
>>>
>>> 145-155 : Constructor with casting
>>>
>>> 171-183 : operator= with casting
>>>
>>>
>>> The corresponding tests for these methods should
>>> be added to the file:
>>>
>>> Insight/Testing/Code/Common/
>>> itkSymmetricSecondRankTensorTest.cxx
>>>
>>>
>>> Please let us know if you run into any problem.
>>>
>>>
>>> It will be nice if you could start trying this code soon,
>>> as we may move into cutting the ITK 3.18 release
>>> as soon as we sort out the licensing issues of the
>>> numerical libraries.
>>>
>>>
>>> Thanks
>>>
>>>
>>> Luis
>>>
>>>
>>>
>>> --------------------------------------
>>> On Tue, Mar 16, 2010 at 2:26 AM, Karthik Krishnan
>>> <karthik.krishnan at kitware.com> wrote:
>>>
>>>
>>>> For FixedArray, Vector, RGBPixel classes, we've already templated the
>>>> operator= and constructor classes to provide implicit casting
>>>> operations and they seem to work well.
>>>>
>>>> Thanks
>>>> --
>>>> karthik
>>>>
>>>> On Mon, Mar 15, 2010 at 4:43 PM, Luis Ibanez <luis.ibanez at kitware.com>
>>>> wrote:
>>>>
>>>>
>>>>> Hi Brad,
>>>>>
>>>>> I can't think of a drawback for the explicit constructors
>>>>> right now,... (but I may just be low in coffee). :-/
>>>>>
>>>>>
>>>>> I worry about the "invisibility" of these methods, and how
>>>>> easy it is to add them, and how hard it will be to remove
>>>>> them if something goes wrong with them.
>>>>>
>>>>>
>>>>> I still would vote for the "CastFrom()" methods, but if
>>>>> enough developers are happy with explicit constructors
>>>>> and are willing to help with the maintenance of the code
>>>>> for the next decade, then I'm happy to follow the crowd. :-)
>>>>>
>>>>>
>>>>> Luis
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> -----------------------------------------------------------------------------
>>>>> On Mon, Mar 15, 2010 at 4:03 PM, Bradley Lowekamp
>>>>> <blowekamp at mail.nih.gov> wrote:
>>>>>
>>>>>
>>>>>> Hello Luis,
>>>>>>
>>>>>> Thanks for the info regarding the itk::Array, I forgot the usefulness
>>>>>> of all the inherited methods!
>>>>>>
>>>>>>
>>>>>> In regards to this, what is the down side to using an explicit
>>>>>> constructor? For example:
>>>>>>
>>>>>> explicit
>>>>>> template<TOtherValueType>
>>>>>> SymmetricSecondRankTensor( const
>>>>>> SymmetricSecondRankTensor<TOtherValueType> &);
>>>>>>
>>>>>> This uses standard conventions along with requiring the
>>>>>> contractor/conversion to be explicitly defined?
>>>>>>
>>>>>> Making the following valid code:
>>>>>>
>>>>>> Float3DTensorType floatTensor2(intTensor);
>>>>>> floatTensor2 = Float3DTensorType(intTensor);
>>>>>>
>>>>>> But not:
>>>>>> floatTensor2 = intTensor;
>>>>>>
>>>>>> Just a thought on an alternative approach....
>>>>>>
>>>>>> Thanks,
>>>>>> Brad
>>>>>>
>>>>>> p.s.
>>>>>> This is my understanding of C++, compilers and standards may differ :)
>>>>>>
>>>>>>
>>>>>> On Mar 15, 2010, at 3:45 PM, Luis Ibanez wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>> Hi Luke,
>>>>>>>
>>>>>>> Historically we have been cautious when adding implicit
>>>>>>> conversions, since once that syntactic sugar is introduced
>>>>>>> in the code, it is very easy to get unintended consequences
>>>>>>> particularly when using templated code.
>>>>>>>
>>>>>>>
>>>>>>> For the case of the itkPoint we were a lot more explicit, and
>>>>>>> added a method:
>>>>>>>
>>>>>>>
>>>>>>> itk::Point<T>::CastFrom( const itk::Point< R > & otherPoint );
>>>>>>>
>>>>>>>
>>>>>>> See the definition in:
>>>>>>>
>>>>>>> Insight/Code/Common/itkPoint.h
>>>>>>>
>>>>>>> in lines 212-221:
>>>>>>>
>>>>>>> /** Copy from another Point with a different representation type.
>>>>>>> * Casting is done with C-Like rules */
>>>>>>> template < typename TCoordRepB >
>>>>>>> void CastFrom( const Point<TCoordRepB,NPointDimension> & pa )
>>>>>>> {
>>>>>>> for(unsigned int i=0; i<NPointDimension; i++ )
>>>>>>> {
>>>>>>> (*this)[i] = static_cast<TCoordRep>( pa[i] );
>>>>>>> }
>>>>>>> }
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> I would suggest that we follow this Explicit approach for
>>>>>>> the Tensor classes.
>>>>>>>
>>>>>>>
>>>>>>> Would you like to give it a try at implementing and testing
>>>>>>> the CastFrom() methods for these classes ?
>>>>>>>
>>>>>>>
>>>>>>> Thanks
>>>>>>>
>>>>>>>
>>>>>>> Luis
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> ----------------------------------------------------------------------------------
>>>>>>> On Fri, Mar 5, 2010 at 2:00 PM, Luke Bloy <luke.bloy at gmail.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>> The classes itkDiffusionTensor3D and itkSymmetricSecondRankTensor
>>>>>>>> don't have
>>>>>>>> constructors that enable casting. (
>>>>>>>> http://www.itk.org/Bug/view.php?id=10323
>>>>>>>> ) so code such as the following fails.
>>>>>>>>
>>>>>>>> typedef itk::SymmetricSecondRankTensor<int,3> Int3DTensorType;
>>>>>>>> typedef itk::SymmetricSecondRankTensor<float,3> Float3DTensorType;
>>>>>>>> typedef itk::SymmetricSecondRankTensor<double,3>
>>>>>>>> Double3DTensorType;
>>>>>>>> Int3DTensorType intTensor(1);
>>>>>>>>
>>>>>>>> //Test constructors
>>>>>>>> Float3DTensorType floatTensor(intTensor);
>>>>>>>> Double3DTensorType doubleTensor(floatTensor);
>>>>>>>>
>>>>>>>> //test Assignment
>>>>>>>> Float3DTensorType floatTensor2 = intTensor;
>>>>>>>>
>>>>>>>>
>>>>>>>> adding templated constructors and assignment operators will allow
>>>>>>>> this type
>>>>>>>> of behavior.
>>>>>>>>
>>>>>>>> Is there a reason these constructors have been not been defined?
>>>>>>>>
>>>>>>>> -Luke
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> Powered by www.kitware.com
>>>>>>>>
>>>>>>>> Visit other Kitware open-source projects at
>>>>>>>> http://www.kitware.com/opensource/opensource.html
>>>>>>>>
>>>>>>>> Kitware offers ITK Training Courses, for more information visit:
>>>>>>>> http://kitware.com/products/protraining.html
>>>>>>>>
>>>>>>>> Please keep messages on-topic and check the ITK FAQ at:
>>>>>>>> http://www.itk.org/Wiki/ITK_FAQ
>>>>>>>>
>>>>>>>> Follow this link to subscribe/unsubscribe:
>>>>>>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Powered by www.kitware.com
>>>>>>>
>>>>>>> Visit other Kitware open-source projects at
>>>>>>> http://www.kitware.com/opensource/opensource.html
>>>>>>>
>>>>>>> Kitware offers ITK Training Courses, for more information visit:
>>>>>>> http://kitware.com/products/protraining.html
>>>>>>>
>>>>>>> Please keep messages on-topic and check the ITK FAQ at:
>>>>>>> http://www.itk.org/Wiki/ITK_FAQ
>>>>>>>
>>>>>>> Follow this link to subscribe/unsubscribe:
>>>>>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>>>>>
>>>>>>>
>>>>>>
>>>>> _______________________________________________
>>>>> Powered by www.kitware.com
>>>>>
>>>>> Visit other Kitware open-source projects at
>>>>> http://www.kitware.com/opensource/opensource.html
>>>>>
>>>>> Kitware offers ITK Training Courses, for more information visit:
>>>>> http://kitware.com/products/protraining.html
>>>>>
>>>>> Please keep messages on-topic and check the ITK FAQ at:
>>>>> http://www.itk.org/Wiki/ITK_FAQ
>>>>>
>>>>> Follow this link to subscribe/unsubscribe:
>>>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>>>
>>>>>
>>>>>
>>> _______________________________________________
>>> Powered by www.kitware.com
>>>
>>> Visit other Kitware open-source projects at
>>> http://www.kitware.com/opensource/opensource.html
>>>
>>> Kitware offers ITK Training Courses, for more information visit:
>>> http://kitware.com/products/protraining.html
>>>
>>> Please keep messages on-topic and check the ITK FAQ at:
>>> http://www.itk.org/Wiki/ITK_FAQ
>>>
>>> Follow this link to subscribe/unsubscribe:
>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>
>>>
More information about the Insight-developers
mailing list