"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."
shntnu
commented
6 years ago
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."