Skip to content

[Type] Introduce function to compute the determinant of any square matrix#5877

Open
alxbilger wants to merge 4 commits intosofa-framework:masterfrom
alxbilger:generalizeddeterminant
Open

[Type] Introduce function to compute the determinant of any square matrix#5877
alxbilger wants to merge 4 commits intosofa-framework:masterfrom
alxbilger:generalizeddeterminant

Conversation

@alxbilger
Copy link
Contributor

  • tests

By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).


Reviewers will merge this pull-request only if

  • it builds with SUCCESS for all platforms on the CI.
  • it does not generate new warnings.
  • it does not generate new unit test failures.
  • it does not generate new scene test failures.
  • it does not break API compatibility.
  • it is more than 1 week old (or has fast-merge label).

@alxbilger alxbilger added pr: enhancement About a possible enhancement pr: fast merge Minor change that can be merged without waiting for the 7 review days pr: status to review To notify reviewers to review this pull-request labels Jan 21, 2026
for (size_t j = 0; j < N; ++j)
{
if (j == p) continue;
submat(i - 1, colIndex++) = mat(i, j);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be more efficient to only create a vector of col and row indices that is reduced of one element for each determinant development ? So in the end your 'submatrix' is like a view on the original matrix, instead of copying the same values multiple times ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not try to reach high performances because I don't have the need. Currently, I use it only in init().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would your changes be easily introduced @bakpaul ?
if yes, we can add it inside this PR
else, it could be done in a later one

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think so, I just need to take time to do it. Give me one week ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fredroy fredroy force-pushed the generalizeddeterminant branch from 6ad8416 to 2a4b912 Compare January 23, 2026 02:08
@hugtalbot hugtalbot removed the pr: fast merge Minor change that can be merged without waiting for the 7 review days label Jan 30, 2026
@alxbilger
Copy link
Contributor Author

@bakpaul I finally changed the algorithm for a Gaussian elimination. Neither your code or mine worked on a 12x12 matrix. It hanged infinitely.

I also implemented benchmarks for this function: alxbilger/SofaBenchmark#44. Here is a possible result:

Gaussian elimination:

------------------------------------------------------------------------------------------
Benchmark                                                Time             CPU   Iterations
------------------------------------------------------------------------------------------
BM_Matrix_typemat_determinant<float, 3>/512           18.5 us         16.3 us        34462
BM_Matrix_typemat_determinant<float, 3>/1024          31.6 us         30.5 us        23579
BM_Matrix_typemat_determinant<float, 3>/2048          62.3 us         55.8 us        11200
BM_Matrix_typemat_determinant<double, 3>/512          15.9 us         14.3 us        44800
BM_Matrix_typemat_determinant<double, 3>/1024         31.5 us         30.7 us        22400
BM_Matrix_typemat_determinant<double, 3>/2048         62.2 us         58.6 us        11200
BM_Matrix_typemat_determinant<float, 4>/512           73.3 us         71.5 us         8960
BM_Matrix_typemat_determinant<float, 4>/1024           148 us          148 us         4978
BM_Matrix_typemat_determinant<float, 4>/2048           296 us          276 us         2489
BM_Matrix_typemat_determinant<double, 4>/512          72.5 us         67.0 us         7467
BM_Matrix_typemat_determinant<double, 4>/1024          146 us          144 us         4978
BM_Matrix_typemat_determinant<double, 4>/2048          311 us          301 us         2489
BM_Matrix_typemat_determinant<float, 12>/512          1485 us         1343 us          640
BM_Matrix_typemat_determinant<float, 12>/1024         2887 us         2790 us          224
BM_Matrix_typemat_determinant<float, 12>/2048         5387 us         5000 us          100
BM_Matrix_typemat_determinant<double, 12>/512         1425 us         1255 us          498
BM_Matrix_typemat_determinant<double, 12>/1024        2658 us         2511 us          280
BM_Matrix_typemat_determinant<double, 12>/2048        5455 us         5000 us          100

BM_Matrix_eigenmat_determinant<float, 3>/512          2.11 us         2.03 us       407273
BM_Matrix_eigenmat_determinant<float, 3>/1024         3.75 us         3.54 us       172308
BM_Matrix_eigenmat_determinant<float, 3>/2048         8.20 us         7.67 us       112000
BM_Matrix_eigenmat_determinant<double, 3>/512         1.90 us         1.76 us       320000
BM_Matrix_eigenmat_determinant<double, 3>/1024        4.21 us         3.84 us       179200
BM_Matrix_eigenmat_determinant<double, 3>/2048        7.59 us         7.50 us        89600
BM_Matrix_eigenmat_determinant<float, 4>/512          5.61 us         5.02 us       112000
BM_Matrix_eigenmat_determinant<float, 4>/1024         9.94 us         9.42 us        89600
BM_Matrix_eigenmat_determinant<float, 4>/2048         20.6 us         19.9 us        34462
BM_Matrix_eigenmat_determinant<double, 4>/512         4.89 us         4.43 us       144516
BM_Matrix_eigenmat_determinant<double, 4>/1024        10.5 us         10.5 us        64000
BM_Matrix_eigenmat_determinant<double, 4>/2048        21.3 us         18.4 us        37333
BM_Matrix_eigenmat_determinant<float, 12>/512         1755 us         1674 us          560
BM_Matrix_eigenmat_determinant<float, 12>/1024        3266 us         2860 us          224
BM_Matrix_eigenmat_determinant<float, 12>/2048        6458 us         5781 us          100
BM_Matrix_eigenmat_determinant<double, 12>/512        1126 us         1001 us          640
BM_Matrix_eigenmat_determinant<double, 12>/1024       2691 us         2613 us          299
BM_Matrix_eigenmat_determinant<double, 12>/2048       5246 us         5022 us          112

For small matrices, even 3x3 we are way slower than Eigen.

@bakpaul
Copy link
Contributor

bakpaul commented Feb 5, 2026

Well it makes sense, thinking about it now, it was in n! complexity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: enhancement About a possible enhancement pr: status to review To notify reviewers to review this pull-request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants