New user self-registration is currently disabled. Please email eigen-core-team @ if you need an account.
Bug 1283 - Bad assertion error in debug mode and wrong result in release mode while evaluating product involving dynamic-size block
Bad assertion error in debug mode and wrong result in release mode while eval...
Product: Eigen
Classification: Unclassified
Component: Core - matrix products
3.3 (current stable)
All All
: Normal Crash
Assigned To: Nobody
Depends on:
Blocks: 3.3
  Show dependency treegraph
Reported: 2016-08-29 20:03 UTC by twan_koolen
Modified: 2016-08-31 13:49 UTC (History)
2 users (show)

Program demonstrating issue (1.31 KB, text/x-csrc)
2016-08-29 20:03 UTC, twan_koolen
no flags Details

Description twan_koolen 2016-08-29 20:03:13 UTC
Created attachment 725 [details]
Program demonstrating issue

See attached program, which triggers an assertion error in debug mode. Assertion error happens when evaluating the product of a dynamically-sized block of a row vector and another matrix into a preallocated matrix (of the correct size).
Comment 1 twan_koolen 2016-08-30 12:58:36 UTC
Also, the results are wrong (with no error or crash) when compiled in release mode (compare c to d in attached program).
Comment 2 Christoph Hertzberg 2016-08-30 13:34:10 UTC
The problem is in
  product_evaluator<Product<Lhs, Rhs, LazyProduct>, 
      ProductTag, DenseShape, DenseShape>::coeff(Index) const
where (src/Core/ProductEvaluator, line 535):
  const Index row = RowsAtCompileTime == 1 ? 0 : index;
But in your case RowsAtCompileTime is Dynamic, so the wrong choice is made.
A quick-fix would be to use MaxRowsAtCompileTime here, though I'm not entirely sure if this breaks in other situations.

Btw: Strangely, it also works with
  d.noalias() = a_block * b;
In this case the coeff(Index, Index) method of the product_evaluator is called (for reasons not obvious to me).

Also, this worked in 3.2 --> block3.3
Comment 3 Gael Guennebaud 2016-08-31 06:41:03 UTC
It's rather unusual to call block on a vector. Anyway, here is a quick fix:

Let me write a full unit test before closing the bug.
Comment 4 twan_koolen 2016-08-31 06:49:09 UTC
I agree, but somebody used this in our codebase and it broke when we updated to the most recent version.

Thanks for the fix!
Comment 5 Gael Guennebaud 2016-08-31 13:49:30 UTC
Sure, I was just looking for an excuse on the lack an appropriate unit test, fixed now:
Summary:     Bug 1283: add regression unit test

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