[Insight-developers] Casting constructors for DTI and Sym Mat pixeltypes
Luis Ibanez
luis.ibanez at kitware.com
Tue Mar 16 19:30:10 EDT 2010
Hi Luke,
The new code is failing in Visual Studio 6.0:
http://www.cdash.org/CDash/viewBuildError.php?buildid=563721
C:\Dashboards\My
Tests\InsightContinuous\Code\Common\itkSymmetricSecondRankTensor.h(119)
: fatal error C1001: INTERNAL COMPILER ERROR
Do you have access to this compiler ?
Please let us know,
Thanks
Luis
--------------------------------------------
On Tue, Mar 16, 2010 at 4:00 PM, Luke Bloy <luke.bloy at gmail.com> wrote:
> 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