[Insight-developers] Casting constructors for DTI and Sym Mat pixeltypes
Luis Ibanez
luis.ibanez at kitware.com
Wed Mar 17 17:20:38 EDT 2010
Hi Luke,
Thanks for keeping an eye on the Dashboard.
I'll catch up with these two compilers tomorrow.
Given that similar code is working for the FixedArray,
we should be able to find a combination for the
tensors classes as well.
Luis
-----------------------------------------------------------------------------------
On Wed, Mar 17, 2010 at 8:28 AM, Luke Bloy <luke.bloy at gmail.com> wrote:
> Hi Luis,
>
> It seems like SunOS-CC-5.6
> <http://www.cdash.org/CDash/buildSummary.php?buildid=564217> also isn't
> happy with the code i submitted.
>
> http://www.cdash.org/CDash/viewBuildError.php?buildid=564217
>
> Unfortunatly, i don't have access to that compiler either.
>
> -Luke
>
> Luis Ibanez wrote:
>>
>> 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