2d head plot connectivity graph

dcxSt commented 3 years ago

Describe the new feature or enhancement

It's a viz feature that enables you to display a visualization of electrode connectivity. To be used in conjunction with the circle.py feature. I couldn't find this feature that my employer requested I implement anywhere in the mne library so I made it myself.

Describe your proposed implementation

Describe possible alternatives

Could be implemented with or without the heatmaps.

Additional comments

The heatmaps require the seaborn library the way I implemented them, and the head-graphs require the networkx library. These features have also been implemented with sliders so that the user can play with the thresholds --thresholding works differently from circle in that you don't specify the number of connections to display you threshold based on connectivity measure, but it would be fairly easy to add that as an option.

I would have to check with my employer if it's okay before adding this feature to the mne, I think they will be chill, besides we don't have a legal contract anyway but I would want to check first so as not to break trust.

welcome[bot] commented 3 years ago

Hello! 👋 Thanks for opening your first issue here! ❤️ We will try to get back to you soon. 🚴🏽‍♂️

drammock commented 3 years ago

Thanks for the suggestion! We already have plot_sensors_connectivity, have you seen this example?

https://mne.tools/stable/auto_examples/connectivity/sensor_connectivity.html

Can you take a look at that and if you think it's missing some functionality that your approach covers, then explain what is different between what you're proposing versus what we already have?

dcxSt commented 3 years ago

Yes this was what I tried first

I had issues with the back-end: some code that worked on my mac wasn't working on my coworkers' windows and vice-versa, this is 2D only and there won't be any backend problems. It was especially buggy in jupyter notebooks.
it's a 3D plot, my employer didn't like it and wanted something beautiful and 2D to put in diagnostic reports --I imagine if he has use for it others may also have use for it
with sensor_connectivity it's tricky to display many connectivity matrices side-by-side, it would be helpful to have this functionality for the sake of comparing different frequency bands along-side each other (see screenshots), or multiple patients side by side
there's a sketch of a little head, nose and ears which are quite cute and help orient you
the adjacent heat-maps

This new function would be similar to ni learn's plot_connectome but with only a top view, although it would be straight-forward to implement side-views too.

larsoner commented 3 years ago

I had issues with the back-end: some code that worked on my mac wasn't working on my coworkers' windows and vice-versa, this is 2D only and there won't be any backend problems. It was especially buggy in jupyter notebooks.

it's a 3D plot, my employer didn't like it and wanted something beautiful and 2D to put in diagnostic reports --I imagine if he has use for it others may also have use for it

I could see how having 2D plotting would be preferable in some/many cases

with sensor_connectivity it's tricky to display many connectivity matrices side-by-side, it would be helpful to have this functionality for the sake of comparing different frequency bands along-side each other (see screenshots), or multiple patients side by side

This to me suggests that we need a 2D plotting function that works with a single sensor connectivity matrix, and it should take an axes argument. Plotting multiple matrices means multiple calls, just with different axes arguments. This is how we usually handle multiple plotting requests I think...

This new function would be similar to ni learn's plot_connectome but with only a top view, although it would be straight-forward to implement side-views too.

Could we have the code wrap to plot_connectome under the hood? The more we can share code / depend on nilearn and just write shims around it (e.g., just to plot a head with sensors for example) the better!

agramfort commented 3 years ago

maybe we can just extend plot_sensors_connectivity to have a 2D matplotlib version?

dcxSt commented 3 years ago

maybe we can just extend plot_sensors_connectivity to have a 2D matplotlib version?

@agramfort Certainly we can recycle the code, in fact that's what I did to some extent, it would probably have to be in a different function though, something like plot_sensors_connectivity_2d, unless you're suggesting modifying plot_sensors_connectivity so that it has a 2d plotting option, in which case there would be a 2d plotting function in the _3D.py file which may be bad ?

Could we have the code wrap to plot_connectome under the hood? The more we can share code / depend on nilearn and just write shims around it (e.g., just to plot a head with sensors for example) the better!

@larsoner huh, I didn't know this was desirable (I'm not very experienced to be honest). I can see how using nilearn functions would be good for maintenance reasons. The reason I personally didn't use it was because it was ugly, nilearn's glass brain makes it look like the graph is inside of the brain, it looks like they're trying to make a 2d plot look 3d which is not ideal, but if you can live with that maybe it is better to use plot_connectome (line 1070). I essentially copied the design of an old report my boss showed me, the plots there had been generated by some matlab proprietary code. Also I found networkx graphs quite flexible and straight-forward, whereas nilearn's graphs seem more complicated (line 2092) .

This to me suggests that we need a 2D plotting function that works with a single sensor connectivity matrix, and it should take an axes argument. Plotting multiple matrices means multiple calls, just with different axes arguments. This is how we usually handle multiple plotting requests I think...

I'm also thinking we could have a function built ontop of that that takes as it's argument a connectivity 3-tensor (the type that is returned by spectral_connectivity) for plotting multiple connectivity bands side by side (like in the screenshots). This kind of function would also be quite useful to have for the circle.py -- but perhaps this is not something that ought to be included in the library? Thoughts ?

drammock commented 3 years ago

To me the best option seems to be expanding plot_sensors_connectivity to have a kind option (mne.viz.plot_sensors has kind='3d' and kind='topomap' and kind='select', but I don't think we need "select" here). We can do the heatmap without relying on Seaborn (currently it's not a dependency) and I'd argue we should do the sensors without nilearn (better to have consistent visual style with our other 2D sensor plots). I'd also try to do it without networkx if we can... ultimately it won't make a difference to the end-user so again I'd rather avoid the extra dependency.

in which case there would be a 2d plotting function in the _3D.py file which may be bad ?

We can always move things around if it makes sense to do so. The API entry point is just mne.viz so it would be fine for the public function to have both 2D and 3D options, and under-the-hood it can call private functions that are possibly in different files.

I'm also thinking we could have a function built ontop of that that takes as it's argument a connectivity 3-tensor (the type that is returned by spectral_connectivity) for plotting multiple connectivity bands side by side (like in the screenshots).

I'd split that into a separate PR after the first one is done... it could basically be a thin wrapper for the new 2D-capable plot_sensors_connectivity that handled creating the correct number of axes, making sure they all had the right colormap, etc

drammock commented 3 years ago

actually I wonder if we even need to do the heatmap part ourselves? If plot_sensors_connectivity can plot into an existing axes object, then the user can use Seaborn / matplotlib / whatever to generate the heatmap in an adjacent axes if they choose... is there a compelling reason for us to handle it?

dcxSt commented 3 years ago

Okay, is there consensus on whether it's worth implementing? I'd be happy to help.

drammock commented 3 years ago

@larsoner @agramfort are you OK with what I proposed above? namely:

add parameter plot_sensors_connectivity(..., kind='3d') (default, current behavior) and plot_sensors_connectivity(..., kind='topomap') (new behavior, uses matplotlib)
add parameter plot_sensors_connectivity(..., ax=None) (default) that is ignored for 3d plots, and can be a matplotlib.axes.Axes instance for 2d plots
don't implement the heatmaps, because seaborn and matplotlib already handle that adequately

@dcxSt one final note that I forgot to mention earlier is that we're moving our connectivity functions into a separate package, so you'll actually want to make the PR into the mne-connectivity package. All the function names and APIs were migrated so everything should look familiar.

cc @adam2392 who has been overseeing the migration to a separate package

agramfort commented 3 years ago

Sounds good 👌

dcxSt commented 3 years ago

Okay! My situation is fairly unstable at the moment (location-wise / jobs ending soon). It may be ~3 weeks until I can work on this. The first step would be to re-create the the connectivity plots without networkx. Are you okay leaving me in charge of this feature or would you rather I push my code to a little repo for others to look at / use / modify.

dcxSt commented 3 years ago

I just have one comment, I'm not sure if the electrode locations in mne.info["chs"][i]["loc"] are highly standardized or not, are they always given in the same units (e.g. meters)? from a standard reference point and orientation? If so can we affor to hard-code the position and eccentricity of the ellipse / trianlge / trapezium that make the head / nose / ears or is there a smarter way of doing it? I'm thinking either there could be an automatic mechanism for fitting the ellipse to the electrode placements, or else the user could play with two parameters, one for radius, and one for eccentricity of the head, and the nose / ear positions would be calculated automatically from that.

agramfort commented 3 years ago

I just have one comment, I'm not sure if the electrode locations in mne.info["chs"][i]["loc"] are highly standardized or not, are they always given in the same units (e.g. meters)?

yes in meters and in MNE head coordinate system defined from the 3 fiducial/cardinal points.

but just reuse the MNE internal mechanism to map 3D chs locs to 2D for topomaps.

see how our topomap functions work internally

dcxSt commented 3 years ago

you just ignore the z-component to project into 2d right

larsoner commented 3 years ago

you just ignore the z-component to project into 2d right

If you're curious you can dig into _find_topomap_coords (which is what you should probably call):

https://github.com/mne-tools/mne-python/blob/041068d510cc746b0a994609a59213f279be0c81/mne/channels/layout.py#L597

Which eventually does:

https://github.com/mne-tools/mne-python/blob/041068d510cc746b0a994609a59213f279be0c81/mne/channels/layout.py#L730-L736

adam2392 commented 1 year ago

@dcxSt Feel free to open a PR for this if still of interest

mne-tools / mne-connectivity