This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen

Bug 949

Summary: Add static assertion for incompatible scalar types in decompositions
Product: Eigen Reporter: sebastien.varrette
Component: LUAssignee: Nobody <eigen.nobody>
Status: RESOLVED FIXED    
Severity: Wrong Result CC: chtz, gael.guennebaud, jitseniesen
Priority: High    
Version: 3.2   
Hardware: All   
OS: All   
Whiteboard:
Bug Depends on: 750    
Bug Blocks: 558    
Attachments:
Description Flags
Demo
none
Static assertions on wrong scalar types
none
Static assertions on wrong scalar types none

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.