[Insight-developers] Casting constructors for DTI and Sym Mat pixeltypes
Luke Bloy
luke.bloy at gmail.com
Thu Mar 18 08:47:58 EDT 2010
Hi Luis,
Thanks for looking into it.
I committed some changes yesterday that I think should solve the VS6.0
problem. However the SunOS-CC-5.6 is still having problems.
-Luke
Luis Ibanez wrote:
> 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