mne-tools / mne-python

MNE: Magnetoencephalography (MEG) and Electroencephalography (EEG) in Python
https://mne.tools
BSD 3-Clause "New" or "Revised" License
2.69k stars 1.31k forks source link

ENH: agenda for beamformer module #5365

Open britta-wstnr opened 6 years ago

britta-wstnr commented 6 years ago

Let's keep record of what issues and enhancements are still open with the beamformer code (not including visualization etc.). Here is a list of open issues and PRs and other things that need to be done:

And then, there is this older issue about beamformer enhancements: #3853. Some of this is done now, but other things might remain, like the different output types (z-scores etc. - which, however, could also just be computed on the output directly).

Anything missing, @agramfort @larsoner @sarangnemo @wmvanvliet ?

larsoner commented 6 years ago

Sounds reasonable to me

agramfort commented 6 years ago

sounds like a plan !

wmvanvliet commented 6 years ago

Yeah, I agree on all these points.

Regarding projections with DICS: turns out, applying SSPs to a CSD works exactly the same as applying them to a covariance matrix! See this gist: https://gist.github.com/wmvanvliet/fe04c40e27c8f81422596163d41563ca

larsoner commented 6 years ago

I wrote a similar test that failed, but looking back at my code I only left-multiplied by the proj op, grrr. Now it works just fine:

https://gist.github.com/larsoner/5a5f7c53c79416212e28a56292f434d7

britta-wstnr commented 6 years ago

Added storing of source orientations in filters. cc @sarangnemo

britta-wstnr commented 6 years ago

Added speeding up LCMV as brought up by @larsoner in #5135

britta-wstnr commented 6 years ago

@larsoner I added the combination of different sensor types.

larsoner commented 6 years ago

@britta-wstnr the max-power has a sign ambiguity. Any reason not to orient it so that positive is in the direction of the vertex normal? Basically this means taking the max power orientation and multiplying by the sign of the dot product with the vertex normal. It's a principled way to deal with the arbitrary flip.

britta-wstnr commented 6 years ago

@larsoner How would we do that in the volume case, do volume fwd models have that information as well? (Note that beamformers are usually computed for the full source grid, not a surface solution.)

larsoner commented 6 years ago

The normals are defined as up Z in that case IIRC. So it will still work

larsoner commented 6 years ago

Done in https://github.com/wmvanvliet/mne-python/pull/6 which probably makes the most sense to put in with #5447 (which is why I made a PR to that branch)

larsoner commented 5 years ago

@wmvanvliet can you take a look at the list above and update the checkboxes?

wmvanvliet commented 5 years ago

done!

On 7 Nov 2018, at 23:39, Eric Larson notifications@github.com wrote:

@wmvanvliet can you take a look at the list above and update the checkboxes?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

mmagnuski commented 5 years ago

(I just moved unchecked checkboxes to the top to make them more visible)

larsoner commented 5 years ago

Clearing 0.19 milestones related to beamformer, @britta-wstnr let me know if there were things you wanted to get into 0.19

britta-wstnr commented 5 years ago

@larsoner I don't think any of the things here are critical. @wmvanvliet and I have some work planned in October (relating to the Spring Coding Sprint comparison between types) and hopefully will close the pending issues along the way!