nasa / polyfit

34 stars 14 forks source link

No way this program could possibly work. #1

Open smacl61 opened 1 year ago

smacl61 commented 1 year ago

You have a procedure called determinant that calls itself while in a loop. On top of that it calls make2darray over and over endlessly on entering determinant consuming alot of memory eventually leading to memory errors. Also you never release the dynamic allocation.

You can't have procedures calling itself - it's terrible practice. There's a famous programmers site named after the condition it causes - stackoverflow :)

I'd fix it for you but I don't know enough about your algo. I just need a working polyfit in c++.

IanikPlante commented 1 year ago

The procedure determinant calls itself recursively, with a smaller matrix size each time. The typical example given for recursion is the calculation of a factorial number. I have not tested the code with large datasets, but for small datasets I am not aware of memory errors.

The code should work. However, I had problems with number precision in some cases, even if double precision numbers are used. If you have specific data for which the code is not working, I can have a look at it.

leaguecn commented 6 months ago

Hi, guys! I have one alternative method.

// add the eigen header files
#include <Eigen/SVD>
#include <Eigen/Dense>

// replace the determinant solver as following code
double determinant(double **a, const size_t k) {

    double det = 0.;
    if (k == 1) return (a[0][0]);    

    Eigen::MatrixXd A(k, k);
    for (size_t i=0; i<k; i++){
      for (size_t j=0; j<k; j++){
        A(i, j) = a[i][j];
      }
    }
    det = A.determinant();
    return (det);
}
SoothingMist commented 2 weeks ago

When attempting to compile this code in Visual Studio, a good many errors and warnings are generated.