Skip to content
This repository has been archived by the owner on Sep 13, 2024. It is now read-only.

Minor cleanup of the correlation calculation #1

Open
shntnu opened this issue Feb 14, 2018 · 0 comments
Open

Minor cleanup of the correlation calculation #1

shntnu opened this issue Feb 14, 2018 · 0 comments

Comments

@shntnu
Copy link
Member

shntnu commented Feb 14, 2018

Daniel suggested that

"As for the code of the covariance, I have been reading the code and it seems
correct to me. The only comments I would have are the following:

Variable n (declared in line 6) is redundant because inside the loop it has
the same value as "i", and outside the loop (lines 22 and 25) has the same
value as "size".
Checking the size in functions "online_covar" and "two_pass_covar" could be
done before the loop. That way, for size=1 the loop wouldn't execute, saving
a few cycles. However the performance impact of this is very small, I don't
think there is any problem as long as these functions are not called
intensively with 1-length vectors (e.g. as base case for a recursive
function).

I would think comparing the result with that obtained by the R function is
indeed sufficient. Maybe another test would be passing a very large matrix
with some wildly different values in it (to facilitate numeric unstability)
but with known covariances. However I suspect the size would have to be VERY
large to cause problems to the R implementation too."

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

No branches or pull requests

1 participant