New user self-registration is disabled due to spam. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Before reporting a bug, please make sure that your Eigen version is up-to-date!
Bug 813 - Improving static_asserts
Summary: Improving static_asserts
Status: NEW
Alias: None
Product: Eigen
Classification: Unclassified
Component: Core - general (show other bugs)
Version: unspecified
Hardware: All All
: Normal Unknown
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-28 12:45 UTC by Gonzalo BG
Modified: 2014-05-28 15:13 UTC (History)
3 users (show)



Attachments

Description Gonzalo BG 2014-05-28 12:45:47 UTC
Static asserts in user code read like this:

eigen3/Eigen/src/Core/PlainObjectBase.h:654:7: error: static_assert failed "INVALID_MATRIX_TEMPLATE_PARAMETERS"
      EIGEN_STATIC_ASSERT((EIGEN_IMPLIES(MaxRowsAtCompileTime==1 && MaxColsAtCompileTime!=1, (Options&RowMajor)==RowMajor)

First of all: 

- the whole condition of the static_assert is not shown by current compilers (it has to be all in a single line).
- there is no documentation for EIGEN_IMPLIES. What does it do? I needed to grep within eigen's code to find that:

#define EIGEN_IMPLIES(a,b) (!(a) || (b))

I find it useful to know what the static assert is doing since "INVALID_MATRIX_TEMPLATE_PARAMETERS" doesn't tell me which thing exactly did I got wrong.

Maybe the right thing would be to split the static_assert into smaller ones, such that, you get more informative error messages.
Comment 1 Christoph Hertzberg 2014-05-28 14:06:05 UTC
(In reply to comment #0)
> - the whole condition of the static_assert is not shown by current compilers
> (it has to be all in a single line).

That is very compiler specific. E.g. gcc does not show any part of the line that causes the assertion. (It does show the line-numbers, however)
In that particular case, would it help you if we added comments like
  // Row-vectors must be RowMajor
after each line?

> - there is no documentation for EIGEN_IMPLIES. What does it do? 

EIGEN_IMPLIES(a,b) is true if a implies b. Where would you look for documentation of that macro? It's actually mostly intended for internal use only, which (I guess) is why it was never documented.

> I needed to grep within eigen's code to find that:

I simply press the "open declaration" hotkey in my IDE to find that ...

> I find it useful to know what the static assert is doing since
> "INVALID_MATRIX_TEMPLATE_PARAMETERS" doesn't tell me which thing exactly did I
> got wrong.
> 
> Maybe the right thing would be to split the static_assert into smaller ones,
> such that, you get more informative error messages.

Yes, that would definitely improve the usefulness of static assertions (I'm not sure on the impact of compilation time). Alternatively, we should at least document those assertions which are not self-explanatory.

And regarding the issue that caused your problem, I'm all for reconsidering Bug 416 ...
Comment 2 Gonzalo BG 2014-05-28 14:17:33 UTC
>would it help you if we added comments like
 > // Row-vectors must be RowMajor
>after each line?

Yes it would, but see below:

During debugging I replaced the whole thing with:

   static_assert(!(MaxRowsAtCompileTime == 1 && MaxColsAtCompileTime != 1) || ((Options&Eigen::RowMajor) == Eigen::RowMajor), "max rows at compile-time can't be 1 if max cols at compile time is not 1 unless the matrix has row major order");

  static_assert(!(MaxColsAtCompileTime == 1 && MaxRowsAtCompileTime != 1) || ((Options&Eigen::RowMajor) == 0), "col major matrix: max cols at compile time can be one only if max rows at compile time is different than 1");

  static_assert(RowsAtCompileTime == Eigen::Dynamic || RowsAtCompileTime >= 0,
                "rows at compile time must either be dynamic or greater equal zero");
  static_assert(ColsAtCompileTime == Eigen::Dynamic || ColsAtCompileTime >= 0,
                "cols at compile time must either be dynamic or greater equal zero");
  static_assert(MaxRowsAtCompileTime == Eigen::Dynamic || MaxRowsAtCompileTime >= 0,
                "max rows at compile time must either be dynamic or greater equal zero");
  static_assert(MaxColsAtCompileTime == Eigen::Dynamic || MaxColsAtCompileTime >= 0,
                "max cols at compile time must either be dynamic or greater equal zero");

  static_assert(MaxRowsAtCompileTime == RowsAtCompileTime || RowsAtCompileTime == Eigen::Dynamic,
                "c");

  static_assert(MaxColsAtCompileTime == ColsAtCompileTime || ColsAtCompileTime == Eigen::Dynamic,
                "d");

  static_assert((Options & (Eigen::DontAlign|Eigen::RowMajor)) == Options, "invalid options");

This did help, a lot. It is neither complete, nor I am sure that the error messages are the right ones, but it might be a starting point. 

In my case, I was passing as option to an Eigen::Matrix Eigen::ColMajor | Eigen::Align, instead of Eigen::AutoAlign... One is for Map, the other is for Matrix. So Options was completely messed-up. Maybe this should also be checked, and Align/AutoAlign should have very incompatible values.

>EIGEN_IMPLIES(a,b) is true if a implies b. Where would you look for
>documentation of that macro? It's actually mostly intended for internal use
>only, which (I guess) is why it was never documented.

If it is intended for internal use I shouldn't see it in the error messages. Otherwise, a Doxygen page explaining what the macros that appear in the error messages is neough. I think it is very useful to see the condition that failed, in particular, if the error messages don't tell you anything.

> I simply press the "open declaration" hotkey in my IDE to find that ...

I guess that the mental block comes from "having to look it up", and not from the process of looking it up in itself.
Comment 3 Gael Guennebaud 2014-05-28 14:59:32 UTC
Arbitrary strings only work when C++11 (or some compiler extensions) is enabled, that is why some "error messages" are not completely self explanatory.

Actually most user will only see "INVALID_MATRIX_TEMPLATE_PARAMETERS" and not the condition.
Comment 4 Gonzalo BG 2014-05-28 15:13:34 UTC
>Arbitrary strings only work when C++11 (or some compiler extensions) is
enabled, that is why some "error messages" are not completely self explanatory.

No. The problem in this case is using a single static assert to cover 9 possible error conditions. If 9 different static asserts would have been used instead, you would be able to get a readable error message even in C++03.

Since I am compiling with C++ >= 11 I used static_assert to work around this issue but C++ >= 11 is not necessary to offer better error messages in this case.

> Actually most user will only see "INVALID_MATRIX_TEMPLATE_PARAMETERS" and not
the condition.

That is implementation defined. For instance, I have always seen a part of the condition.

Note You need to log in before you can comment on or make changes to this bug.