Re: [eigen] Pitfall in SelfAdjointEigenSolver API compared to EigenSolver API

[ Thread Index | Date Index | More Archives ]


sorry for the delay, I've lost your email in the flow.

What is pretty bad is that we kept the bool version in EigenSolver. So
what I suggest is to have both versions in both SelfAdjointEigenSolver
and EigenSolver. In EigenSolver, the bool version would be marked as
deprecated. In SelfAdjointEigenSolver the bool version would issue a
compile time error (unless EIGEN2_SUPPORT is defined). Alternatively,
we could also make this later working and simply put a deprecated
warning. I have no opinion for one or the other option.


On Thu, Apr 28, 2011 at 9:19 PM, Schmidt, Michael
<Michael.Schmidt@xxxxxxxxxxxxxxxxxxx> wrote:
> Dear Eigen-Community,
> having recently updated to Eigen 3.0 (a really great piece of C++
> engineering!), I stumbled over a pitfall related to the new API of the
> SelfAdjointEigenSolver (and GeneralizedSelfAdjointEigenSolver) class, which
> is slightly different than both, the 2.0.x API of SelfAdjointEigenSolver and
> EigenSolver:
> In 3.0, the second parameter of compute-method of the
> SelfAdjointEigenSolvers is an int variable, in which a combination of flags
> can be stored. In contrast, the 2.0.x API of that class as well as the API
> of EigenSolver use a simple bool variable at that place, which is used to
> enable or disable calculation of Eigenvectors on top of the Eigenvalues. The
> problem is, that using a bool with the SelfAdjointEigenSolver in 3.0 does
> compile (casting the bool to 0 or 1, at least in Visual Studio 2008), but it
> doesn’t produce the correct result because “1” does not match the required
> enum value “ComputeEigenvectors”. The same will be true the other way round,
> casting the int to a bool in the EigenSolver case. Luckily, in debug mode
> passing a bool to SelfAdjointEigenSolver::compute is detected by an assert,
> pointing you to the source of the error. However, in release mode the assert
> is disabled and the eigenvectors are simply undefined, what may be hard to
> track down in complex applications. Since this is a problem that may hit
> both, users upgrading from pre 3.0 versions, and users switching from the
> general implementation to the SelfAdjoint one (or similar the other way
> round), I think it would be a good thing to somehow prevent these errors in
> the beginning. One possible solution might be to add a backward compatible
> overload of the compute-method (and the constructor, which can take the same
> parameter) to the SelfAdjointEigenSolver (and possibly an overload with
> int-flags in the EigenSolver), but perhaps there may be other ways e.g. for
> generating a compile-error when the wrong parameter type is used?
> Kind regards,
> Michael

Mail converted by MHonArc 2.6.19+