Re: [eigen] Bugs in Mat::Random, reductions

[ Thread Index | Date Index | More Archives ]

On Thu, Oct 23, 2008 at 4:53 PM, Benoît Jacob <jacob@xxxxxxxxxxxxxxx> wrote:
> On Thursday 23 October 2008 04:01:06 Patrick Mihelich wrote:
>> Btw, I think there is a typo in the "Basis vectors" code box: UnixX()
>> instead of UnitX(), etc.
> Gael: with that, you get the trophy of the most l33t l4p5u5 :)


>> Still, I thought of a cute trick you could use to get friendlier error
>> messages. According to the standard, "The default arguments in a member
>> function definition that appears outside of the class definition are added
>> to the set of default arguments provided by the member function declaration
>> in the class definition." We can abuse this to make nicer error messages,
>> like so:
>> [...]
>> Then when the user includes <Eigen/Array>, he effectively gets the expected
>> zero-arg version of rowwise. If he forgets to include it, no zero-arg
>> version exists, so mat.rowwise() should produce a compiler error along the
>> lines of:
> Thanks for the trick... the first thing to notice is that it is only suitable
> for functions that are always inlined. For a non-inlined function, this
> incurs a (very small) overhead and more importantly this affects the
> function's signature.

yes, same here, this is a nice trick, but it is rather intrusive.

> How about this other solution: we let people enable modules by #defining a
> symbol before #including Eigen, so
> #include <Eigen/Array>
> #include <Eigen/Geometry>
> becomes
> #include <Eigen/Eigen>
> you read it well: only one public header. The old public headers stay until
> the next beta to let us time to port existing code. If you need only Core, no
> need to define anything.
> I mean to use these symbols to do something like
> class MatrixBase
> {
>   void someCoreModuleFunc();
>   ...
>   void someArrayModuleFunc();
>    ...
> #endif
> };
> I know that's very unusual, so I don't feel 100% comfortable. What do you guys
> think?

Between the two proposals, I prefer this one, even though, like you,
I'm still a bit puzzled. From the user point of view, what happens if
someone write:


whithout including Eigen/Array ? he will simply get an error stating
something like:

"file X:lineY: error class Cwise<long list of arguments> has no member sin()"

instead of the current:

"in function Y:
 fileX: undefined reference to Cwise<long list of arguments>::sin()"

is it really more explicit ?

So maybe we could mix the 2 approaches. In practice we can define the
macro EIGEN_ARRAY_ONLY like that:

#define you_must_include_array
struct you_must_include_array{};

and declares all "array only" method with:

void someArrayModuleFunc(EIGEN_ARRAY_ONLY);

and for doxygen, we just have to tell it to expand the EIGEN_*_ONLY macros.

then the user will get an explicit error message, and the prototype of
our function remains clean.

what do you think ? at least if we go for the EIGEN_USING_*_MODULE
macros, I think the above trick the right way to go. So the question
is whether everybody is ok with the EIGEN_USING_*_MODULE approach ?

At least I don't have strong arguments against this idea, so currently
I would be ok to go for it.

> This means that in a single project, different .cpp files may see different
> definitions of the MatrixBase class (if they use different modules). Can this
> be a problem?

I don't think that's a problem as long as you don't hide/show virtual
functions that will never happens in our case.

>> I did a straightforward reimplementation of some of the pose estimation
>> code using Eigen2 - it's about half the lines of code, far more readable,
>> and almost twice as fast. Very impressive work you've done; I will be
>> playing with this library more.
> Thank you; on the x86 platform be sure to tell your compiler to enable SSE2
> instructions (-msse2 with GCC) so Eigen uses its vectorized paths (default on
> x86-64).

indeed, I'd be very curious to know which version of the the compiler
you used and whether you had the vectorization enabled ?


Mail converted by MHonArc 2.6.19+