[Insight-developers] Proposal: Changing NumericTraits of vector types to use macros
Gaëtan Lehmann
gaetan.lehmann at jouy.inra.fr
Thu Dec 3 15:06:24 EST 2009
Le 3 déc. 09 à 15:52, Bradley Lowekamp a écrit :
> Gaetan,
>
> As these changes were larger then expected am glad to hear that you
> had a read through. I believe that the min, max, and nonpositiveMin
> methods were not there before for the FixedArray, Vector,
> CovariantVector and Tensor NumericTraits. So I presumed it was just
> result of copying and pasting the macro.
>
> Is there a use for these traits?
> Would it enable these pixel types to use a filter the currently
> couldn't?
>
The main goal is to gain some consistency in the numeric traits.
I have no specific example in mind, but in the past the lack of those
methods for RGBPixel has made impossible the build of some classes.
IIRC, max() and Zero where used as default values for the filter
parameters.
> The VariableLengthVector defined these methods with a Self& argument.
> Should these other types define these additional methods as well?
> Are there filters that could use it?
That's a more difficult question!
My feeling is that it would give more flexibility if those methods
were available in all the traits, and widely used in the toolkit.
Even with that, there are cases where they would be unusable, because
when they are used, the number of pixels is not know yet - to
initialize the members of a filters with default values for example.
So I wouldn't add those ones.
>
> As it is much harder to remove methods from itk then it is to add, I
> am cautious of adding new methods that will not be used. If there is
> a use, we should write a test to make sure it's right, and work the
> expected way.
>
You're right of course - everything should be tested.
I'll search for the use case above, and see if it can be extended to
those types.
Gaëtan
> Brad
>
> On Dec 3, 2009, at 9:23 AM, Gaëtan Lehmann wrote:
>
>>
>> Brad,
>>
>> I've readded the methods max(), min() and NonpositiveMin() to the
>> array traits.
>> I think they were removed by mistake in
>>
>> http://public.kitware.com/cgi-bin/viewcvs.cgi/Code/Common/itkNumericTraitsFixedArrayPixel.h?root=Insight&r1=1.6&r2=1.7&sortby=date
>>
>> , but maybe I'm wrong. Was it done on purpose?
>>
>> Regards,
>>
>> Gaëtan
>>
>>
>> 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?
>>>
>>> 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
>>
>
> ========================================================
> 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/20091203/4ec2ca26/attachment.pgp>
More information about the Insight-developers
mailing list