[Insight-developers] Casting constructors for DTI and Sym Mat pixeltypes
Luis Ibanez
luis.ibanez at kitware.com
Tue Mar 16 21:04:58 EDT 2010
Hi Luke,
What is structurally different between the new code added to the
itkSymmetricSecondRankTensor and the templated constructors
that Karthik pointed out that we already have in itkFixedArray ?
That difference may provide a hint on what must be modified in
order to induce happiness in the Visual Studio 6.0 build....
Luis
---------------------------------------------------------------------------------------
On Tue, Mar 16, 2010 at 7:47 PM, Luke Bloy <luke.bloy at gmail.com> wrote:
> Luis,
>
> I do not have access to that compiler but i suspect the problem is that I
> neglected to remove the non-templated constructor and assignment operators.
>
> I have just removed those methods and am running tests on my gcc box.
>
> I can either commit the changes or if you prefer we can wait until someone
> with a vs 6 box can test the patch.
>
> Sorry for the inconvenience.
>
> Luke
>
> Luis Ibanez wrote:
>>
>> 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