[Insight-developers] Casting constructors for DTI and Sym Mat pixeltypes
Luke Bloy
luke.bloy at gmail.com
Wed Mar 17 08:28:08 EDT 2010
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