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
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.
Ah ok, it definitively makes sense ;) For sure a raised error would help.
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.
(In reply to Gael Guennebaud from comment #3) > Created attachment 555 [details] This appears to be part of the patch to Bug 973?
Created attachment 556 [details] Static assertions on wrong scalar types oops, that one should be right.
*** Bug 750 has been marked as a duplicate of this bug. ***
(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.
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.) ??
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)?
(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.
-- 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.