[Insight-developers] Proposal: Changing NumericTraits of vector types to use macros
Gaëtan Lehmann
gaetan.lehmann at jouy.inra.fr
Tue Dec 1 13:16:00 EST 2009
Le 1 déc. 09 à 18:12, Bradley Lowekamp a écrit :
> In working on this I discovered the follow definition for RGB and
> RGBA pixels:
>
> static const Self NonpositiveMin()
> {
> return NumericTraits< Self >::min();
> }
>
> While not clearly documented any where the purpose of the
> NonpositiveMin, is to have the a value that is less then all other
> valid values for a type ( perhaps defined on a per-component basis).
> Which differs from the min() trait is that
> itk::NumericTraits<float>::min() > 0.
>
>
> This looks like a bug and should be change to:
>
> static const Self NonpositiveMin()
> {
> return Self( NumericTraits< ValueType>::NonpositiveMin() );
> }
>
> Does anyone see a problem?
It also seemed a bit strange to me when I copied it. Your
implementation seems to be the right one.
Gaëtan
>
> Also as I feel that I have the ideas of all the traits in my head
> now. I would like to document them in the generic and unspecialized
> NumericTraits at the beginning of itkNumericTraits.h. I believe
> Doxygen can be told to just document the class in the file and skip
> all the specializations.
>
> Brad
>
>
>
>
> On Nov 30, 2009, at 12:07 PM, Lowekamp, Bradley (NIH/NLM/LHC) [C]
> wrote:
>
>> I will work on these improvements to your work, and try to commit
>> tomorrow morning.
>>
>> Brad
>>
>> On Nov 30, 2009, at 11:26 AM, Gaëtan Lehmann wrote:
>>
>>>
>>> Le 30 nov. 09 à 16:24, Bradley Lowekamp a écrit :
>>>
>>>> Hello Gaetan,
>>>>
>>>> The way you used just one macro for the different traits was very
>>>> nicely done. I had not done that.
>>>>
>>>> Two important differences with your implementation are:
>>>>
>>>> 1) The ElementTraits are public types. I am not sure that these
>>>> are needed for the interface, and may just be an implementation
>>>> detail. But if we actively want them as part of the interface
>>>> that is fine. I was going to make them private so as not to add
>>>> more confusion to the NumericTraits interface.
>>>>
>>>
>>> Yes, good idea. I remove the function
>>>
>>> Self max( const Self & )
>>> Self min( const Self & )
>>>
>>> which were in RGBAPixel traits already, because the where in none
>>> of the traits I was working on, and doesn't seemed directly useful
>>> - I think they come from the numeric traits of VariableLength
>>> vector, so we may want to readd them to uniformize all the numeric
>>> traits.
>>>
>>>> 2) Your macro only uses the total template specialization, where
>>>> as the other implementation conditionally uses partial template
>>>> specialization, so that on most compilers large sized arrays
>>>> could be used even though they were not totally specialized.
>>>
>>> I tried to reproduce as exactly as possible what was already
>>> there, but with some macros. I was not confident enough in how the
>>> specialization is handle in all those compilers to reproduce
>>> exactly what is done in RGBAPixel for example.
>>>
>>> If you are more comfortable than me on that subject, it would be
>>> great to do it your way!
>>>
>>>>
>>>> An Additional point, I have, is that OneValue() and ZeroValue
>>>> should be changed to not use then static member variables. There
>>>> are two reasons for this:
>>>> 1) with partial specialization, there may not be a defined member
>>>> variable location, by initializing it in the function we can get
>>>> around this
>>>> 2) in many cases the compilers are smart enough to optimize away
>>>> certain numeric when the value is known at compile time (inlined
>>>> in the function).
>>>>
>>>
>>> OK, good points - I thought that it may be more efficient that
>>> way, but it seems that I was wrong :-)
>>>
>>> Gaëtan
>>>
>>>
>>>> Brad
>>>>
>>>>
>>>> On Nov 30, 2009, at 4:38 AM, Gaëtan Lehmann wrote:
>>>>
>>>>>
>>>>> Brad, Luis,
>>>>>
>>>>> I have already commited some code on that subject, while working
>>>>> on two long standing bugs in wrapitk - the missing NumericTraits
>>>>> while building with dimension 4, and the empty NumericTraits<
>>>>> FixedArray< * > > classes.
>>>>>
>>>>> I've modified some macros from RGBAPixel numeric traits to
>>>>> refactor the numeric traits of FixedArray, Vector,
>>>>> CovariantVector and SymmetricSecondRankTensor.
>>>>>
>>>>> Regards,
>>>>>
>>>>> Gaëtan
>>>>>
>>>>>
>>>>>
>>>>> Le 28 nov. 09 à 18:09, Luis Ibanez a écrit :
>>>>>
>>>>>> Hi Brad,
>>>>>>
>>>>>> Yes, it is a good idea to propagate the use of Macros for other
>>>>>> NumericTraits.
>>>>>>
>>>>>> We took a historical detour in the development of the
>>>>>> NumericTraits, and
>>>>>> only come to use the convenience macros with the most recently
>>>>>> added
>>>>>> classes. It will be great to back-port this practice to the
>>>>>> initial vector-like
>>>>>> classes.
>>>>>>
>>>>>> As you pointed out, using the macros will result in more
>>>>>> consistent and
>>>>>> maintainable code.
>>>>>>
>>>>>>
>>>>>> Luis
>>>>>>
>>>>>>
>>>>>> ---------------------------------------------------------------------------
>>>>>> On Fri, Nov 27, 2009 at 11:16 AM, Bradley Lowekamp
>>>>>> <blowekamp at mail.nih.gov> wrote:
>>>>>>> Hello,
>>>>>>> The next logical step after adding itk::NumericTraits<long
>>>>>>> long> is to also
>>>>>>> add it to all the vector types that also have numeric traits
>>>>>>> defined. Unfortunately I am lazy, and manually going through
>>>>>>> all these
>>>>>>> files, and close to 5,000 lines of code, to perform copy,
>>>>>>> paste, edit seems
>>>>>>> like too much work.
>>>>>>> Looking at itkNumericTratisVariableLengthVectorPixel.h:
>>>>>>> http://public.kitware.com/cgi-bin/viewcvs.cgi/Code/Common/itkNumericTraitsVariableLengthVectorPixel.h?revision=1.7&root=Insight&view=markup
>>>>>>> This seems to have an elegant macro which both does total
>>>>>>> template
>>>>>>> specialization and partial specialization when needed. It
>>>>>>> seems like much
>>>>>>> less work to implement similar macros in each file. One
>>>>>>> important change I'd
>>>>>>> make to similar macros for other types is to make the
>>>>>>> "ElementTypes" (which
>>>>>>> are needed for bcc) private so as to not add additional traits
>>>>>>> that have not
>>>>>>> been defined as part of the interface.
>>>>>>> As a "good" side effect, this will help maintainability and
>>>>>>> correct errors
>>>>>>> that currently exist in the traits. For example the many
>>>>>>> numeric traits for
>>>>>>> for char vector types still have the char vector type as the
>>>>>>> PrintType,
>>>>>>> which is wrong because it'll print ASCII value instead of
>>>>>>> numbers. There are
>>>>>>> also missing trait types in many vector types as well. For
>>>>>>> example the
>>>>>>> CovariantVector traits are missing the FloatType.
>>>>>>>
>>>>>>> Brad
>>>>>>>
>>>>>>> ========================================================
>>>>>>>
>>>>>>> Bradley Lowekamp
>>>>>>>
>>>>>>> Lockheed Martin Contractor for
>>>>>>>
>>>>>>> Office of High Performance Computing and Communications
>>>>>>>
>>>>>>> National Library of Medicine
>>>>>>>
>>>>>>> blowekamp at mail.nih.gov
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> 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
>>>>>
>>>>> --
>>>>> Gaëtan Lehmann
>>>>> Biologie du Développement et de la Reproduction
>>>>> INRA de Jouy-en-Josas (France)
>>>>> tel: +33 1 34 65 29 66 fax: 01 34 65 29 09
>>>>> http://voxel.jouy.inra.fr http://www.itk.org
>>>>> http://www.mandriva.org http://www.bepo.fr
>>>>>
>>>>
>>>> ========================================================
>>>> Bradley Lowekamp
>>>> Lockheed Martin Contractor for
>>>> Office of High Performance Computing and Communications
>>>> National Library of Medicine
>>>> blowekamp at mail.nih.gov
>>>>
>>>>
>>>
>>> --
>>> Gaëtan Lehmann
>>> Biologie du Développement et de la Reproduction
>>> INRA de Jouy-en-Josas (France)
>>> tel: +33 1 34 65 29 66 fax: 01 34 65 29 09
>>> http://voxel.jouy.inra.fr http://www.itk.org
>>> http://www.mandriva.org http://www.bepo.fr
>>>
>>
>> ========================================================
>> Bradley Lowekamp
>> Lockheed Martin Contractor for
>> Office of High Performance Computing and Communications
>> National Library of Medicine
>> blowekamp at mail.nih.gov
>>
>>
>> <ATT00001..txt>
>
> ========================================================
> Bradley Lowekamp
> Lockheed Martin Contractor for
> Office of High Performance Computing and Communications
> National Library of Medicine
> blowekamp at mail.nih.gov
>
>
--
Gaëtan Lehmann
Biologie du Développement et de la Reproduction
INRA de Jouy-en-Josas (France)
tel: +33 1 34 65 29 66 fax: 01 34 65 29 09
http://voxel.jouy.inra.fr http://www.itk.org
http://www.mandriva.org http://www.bepo.fr
-------------- next part --------------
A non-text attachment was scrubbed...
Name: PGP.sig
Type: application/pgp-signature
Size: 203 bytes
Desc: Ceci est une signature ?lectronique PGP
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20091201/d8fabf4d/attachment.pgp>
More information about the Insight-developers
mailing list