Added variance & rowwise variance (for matricies) and vector variance #3249
Conversation
| Eigen::Map<VectorXt> eigMean = tempVec; | ||
| for(int i = 0; i < x.num_cols; ++i) | ||
| { | ||
| eigSquaredResult.col(i) = eigMean; |
There was a problem hiding this comment.
I don't know if there's a better way to do this (the loop starting on line 299) - I'm trying to make all of the columns of eigMean be equal to tempVec (which holds the rowwise mean for each row)
There was a problem hiding this comment.
Maybe we can avoid having the temporary matrix eigSquaredResult altogether.
|
It's also worth nothing that I haven't implemented a columnwise computation as there's no columnwise mean yet - I can do that in a later PR. |
|
|
||
| T meanVal = mean<Backend::EIGEN3, SGMatrix<T>>::compute(x); | ||
| eigSquaredResult.fill(meanVal); | ||
| eigSquaredResult = eigX - eigSquaredResult; |
There was a problem hiding this comment.
This line could be problematic. Did you make sure that it works?
Also, since this squared result thing is not being returned by the API, it can just be a Eigen3 matrix that you use internally for this method. As long as the API doesn't take or return a non SG type, we're good. See what I mean?
There was a problem hiding this comment.
It works on some small unit test - what problem could arise?
I̶n̶ ̶r̶e̶g̶a̶r̶d̶s̶ ̶t̶h̶e̶ ̶s̶q̶u̶a̶r̶e̶d̶R̶e̶s̶u̶l̶t̶ ̶m̶a̶t̶r̶i̶x̶,̶ ̶a̶r̶e̶ ̶y̶o̶u̶ ̶s̶a̶y̶i̶n̶g̶ ̶I̶ ̶d̶o̶n̶'̶t̶ ̶n̶e̶e̶d̶ ̶i̶t̶?̶ ̶ ̶T̶h̶a̶t̶ ̶m̶a̶k̶e̶s̶ ̶s̶e̶n̶s̶e̶.̶ ̶ ̶B̶u̶t̶ ̶w̶h̶a̶t̶ ̶s̶h̶o̶u̶l̶d̶ ̶I̶ ̶r̶e̶p̶l̶a̶c̶e̶ ̶i̶t̶ ̶w̶i̶t̶h̶?̶ ̶ ̶I̶ ̶t̶h̶o̶u̶g̶h̶t̶ ̶a̶l̶l̶ ̶M̶a̶p̶s̶ ̶h̶a̶v̶e̶ ̶t̶o̶ ̶b̶e̶ ̶i̶n̶i̶t̶i̶a̶l̶i̶s̶e̶d̶ ̶w̶i̶t̶h̶ ̶s̶o̶m̶e̶t̶h̶i̶n̶g̶ ̶a̶n̶y̶w̶a̶y̶.̶ ̶ ̶ I get what to do now.
…lication with squaring
|
@lambday I've removed as some of the temp vectors, but I believe that some of the ones you pointed out (e.g. eigenSquaredResult) are needed. Please refer to the new commit. |
| * either expressed or implied, of the Shogun Development Team. | ||
| */ | ||
|
|
||
| #ifndef SRC_SHOGUN_MATHEMATICS_LINALG_INTERNAL_IMPLEMENTATION_VARIANCE_H_ |
There was a problem hiding this comment.
ehm.....this might be a bit long, no? :)
|
Just saw your last comment and leaving this for now. |
Submitting for issue #3118 .