This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
Bug 949 - Add static assertion for incompatible scalar types in decompositions
Summary: Add static assertion for incompatible scalar types in decompositions
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: LU (show other bugs)
Version: 3.2
Hardware: All All
: High Wrong Result
Assignee: Nobody
URL:
Whiteboard:
Keywords:
: 750 (view as bug list)
Depends on: 750
Blocks: 3.3
  Show dependency treegraph
 
Reported: 2015-02-08 22:04 UTC by sebastien.varrette
Modified: 2019-12-04 14:11 UTC (History)
3 users (show)



Attachments
Demo (2.50 KB, text/x-csrc)
2015-02-08 22:04 UTC, sebastien.varrette
no flags Details
Static assertions on wrong scalar types (3.23 KB, patch)
2015-03-09 17:07 UTC, Gael Guennebaud
no flags Details | Diff
Static assertions on wrong scalar types (22.05 KB, patch)
2015-03-09 21:36 UTC, Gael Guennebaud
no flags Details | Diff

Description sebastien.varrette 2015-02-08 22:04:27 UTC
Created attachment 537 [details]
Demo

For a regular square matrix with n >= 5 rows, the determinant computation returns completely wrong results. I attach a sample code.

For instance for the following matrix: 

```
Eigen::Matrix<int,5,5> C0, C1, C2;
    C0 <<
        1, 2, 3, 4, 5,
        3, 0, 4, 5, 6,
        2, 1, 2, 3, 4,
        0, 0, 0, 6, 5,
        0, 0, 0, 5, 6;
    C1 <<
        0, 0,  3, 1, 1,
        3, 1,  1, 0, 1,
        1, 0,  2, 3, 3,
        3, 1,  2, 3, 3,
        0, 2,  1, 2, 1;
    C2 <<
	1, 1,  1,  1,  1,
	1, 2,  3,  4,  5,
	1, 3,  6, 10, 15,
	1, 4, 10, 20, 35,
	1, 5, 15, 35, 69;
```
Expected det(C0) = 99  (when C0.determinant() = -432
Expected det(C1) = -25 (when C1.determinant() = -54
Expected det(C2) = 0   (when C2.determinant() = 1296
Comment 1 Jitse Niesen 2015-02-09 08:13:03 UTC
Without looking at the code or doing any tests, I would say that the problem is probably that the scalar type is int. One needs division to do an LU decomposition and integer division is not a proper division. We should add a compile-time error to cover this case.
Comment 2 sebastien.varrette 2015-02-09 17:15:47 UTC
Ah ok, it definitively makes sense ;) 
For sure a raised error would help.
Comment 3 Gael Guennebaud 2015-03-09 17:07:57 UTC
Created attachment 555 [details]
Static assertions on wrong scalar types

This patch adds static assertions on the input scalar types of a dozen of dense solvers.
Comment 4 Christoph Hertzberg 2015-03-09 20:43:02 UTC
(In reply to Gael Guennebaud from comment #3)
> Created attachment 555 [details]

This appears to be part of the patch to Bug 973?
Comment 5 Gael Guennebaud 2015-03-09 21:36:06 UTC
Created attachment 556 [details]
Static assertions on wrong scalar types

oops, that one should be right.
Comment 6 Christoph Hertzberg 2015-03-13 21:05:50 UTC
*** Bug 750 has been marked as a duplicate of this bug. ***
Comment 7 Christoph Hertzberg 2015-03-13 21:11:03 UTC
(In reply to Gael Guennebaud from comment #5)
> Created attachment 556 [details]
> Static assertions on wrong scalar types

Patch looks good. Probably, we should also check in the default constructors to catch errors as early as possible.
Comment 8 Gael Guennebaud 2015-03-13 21:24:20 UTC
https://bitbucket.org/eigen/eigen/commits/678c42a8a42a/
Changeset:   678c42a8a42a
User:        ggael
Date:        2015-03-13 20:06:20+00:00
Summary:     Bug 949: add static assertion for incompatible scalar types in dense end-user decompositions.

Do we want to assert on helper decompositions too (Triadiagonal, Hessenberg, etc.) ??
Comment 9 Christoph Hertzberg 2015-03-13 22:06:26 UTC
I think we don't have to add it this to Tridiagonal, etc. as long as they are not supposed to be called from user code. It wouldn't hurt much either, I guess.

Shall we backport this to 3.2 (what the original report was for)?
Comment 10 Gael Guennebaud 2015-03-13 22:34:43 UTC
(In reply to Christoph Hertzberg from comment #9)
> Shall we backport this to 3.2 (what the original report was for)?

done:
https://bitbucket.org/eigen/eigen/commits/c24144a314e4/

For the other decompositions if anyone want to do it, go ahead.
Comment 11 Nobody 2019-12-04 14:11:47 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/949.

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