Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should PearsonsCorrelation#computeCorrelationMatrix() check NaN results? #283

Open
axkr opened this issue Nov 12, 2023 · 1 comment
Open

Comments

@axkr
Copy link
Contributor

axkr commented Nov 12, 2023

Should PearsonsCorrelation#computeCorrelationMatrix() check NaN results here to detect errors earlier in the computation?

double corr = correlation(matrix.getColumn(i), matrix.getColumn(j));

if (Double.isNaN(corr)){
  throw exception
}

Example matrix {{0.0,0.0},{0.0,0.0}}

axkr added a commit to axkr/symja_android_library that referenced this issue Nov 12, 2023
@maisonobe
Copy link
Contributor

When generating this exception, one of the reference test for MillerUpdatingRegression fails (the testPCorr test).
Looking at what this failing test does, it appears that the Pearson correlation matrix indeed as NaN entries in first row and first column (except diagonal element which is 1.0), but these entries are not used.
So I am not sure if we should throw the exception at this low level, as it prevents computing the rest of the matrix as soon as we get the first NaN.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants