[Insight-developers] Proposal: Changing NumericTraits of vector types to use macros

Tue Dec 1 12:15:38 EST 2009

Sure looks like a bug for non integral RGB/RGBA's.

On Tue, Dec 1, 2009 at 12:12 PM, Bradley Lowekamp
<blowekamp at mail.nih.gov> wrote:
> 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
>
>
> _______________________________________________
> 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
>
>