MRG, DOC: Inverse math #7160
MRG, DOC: Inverse math #7160
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7160 +/- ##
==========================================
- Coverage 89.88% 89.71% -0.17%
==========================================
Files 450 450
Lines 81320 81408 +88
Branches 12926 12940 +14
==========================================
- Hits 73091 73034 -57
- Misses 5399 5517 +118
- Partials 2830 2857 +27 |
agramfort
left a comment
agramfort
left a comment
There was a problem hiding this comment.
that's a start. Let me know when to have another look. Sorry it took me so long to look.
| assert stc.subject == 'sample' | ||
| assert stc.data.min() > 0 | ||
| assert stc.data.max() < 10.0 | ||
| assert 2 < stc.data.max() < 5 |
There was a problem hiding this comment.
why did you need to update these tests?
There was a problem hiding this comment.
Didn't need to, but it helps during changes to the inverse code to have them be more precise
|
@agramfort only bf467a2 is new but I rebased + force-pushed, should have addressed all your comments: |
|
FYI I killed Travis because it's going to fail due to failures in |
| as the whitened measurement vector at time *t*. The spatial | ||
| whitening operator :math:`C^{-^1/_2}` is obtained with the help of the | ||
| eigenvalue decomposition | ||
| :math:`C = U_C \Lambda_C^2 U_C^\top` as :math:`C^{-^1/_2} = \Lambda_C^{-1} U_C^\top`. |
There was a problem hiding this comment.
I've always found the notation of the whitener C^{-1/2} confusing because it is not really the usual inverse square root of positive definite matrix. If C = U_C \Lambda_C^2 U_C^\top, the usual definition of C^{-1/2} is U_C \Lambda_C^{-1} U_C^\top which is not the whitener. Is there some reason I'm missing for this def ? otherwise we should just name it W
There was a problem hiding this comment.
Perhaps, but I'd rather sort that notation change out as a separate PR. I'm trying as much as possible to stick to the notation in the existing document, and I suspect that changing this will not be trivial.
|
@hichamjanati I pushed a commit to hopefully address your comments with the exception of the |
|
I'll go ahead and merge this once CIs are happy. BTW @hichamjanati feel free to take a shot at the whitener changes if you want (after merge) :) |
* DOC: Cleaner equations * DOC: Wording
* DOC: Cleaner equations * DOC: Wording
3 participants
Working on making eLORETA have units nAm.Along the wayWriting out some of the inverse derivations and adding equation links so that they are hopefully easier to follow.This does not work, mostly opening PR so that @agramfort can look at where I've messed up the math :)