New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
plot_alignment should show transparent brain when using sEEG electrodes #8445
Comments
@GuillaumeFavelier thoughts? any way to expose such options? |
Yeah, I prefer the result with transparency
|
Currently we make the innermost surface opaque (alpha=1). It's probably fine instead just to always make the innermost surface translucent (alpha=0.75 or something). That way you can see stuff behind it anyway, which is nice. Would that work for your sEEG example? These are the examples where https://mne.tools/dev/generated/mne.viz.plot_alignment.html#examples-using-mne-viz-plot-alignment @kdoelling1919 if you want to make a pull request with this change, if you could make minor improvements to wording if possible (otherwise just add an extra PEP8-compatible newline somewhere) to some of them, it will cause CircleCI to render them so we can see the outputs. Ideally you could pick examples that cover the various uses of In a worst case we can make |
That's wonderful! That's what I want! |
OK. So i'm not totally clear where we've landed. Seems like options are:
I'm happy with any of them. Though I can imagine that if we changed everything to .75 (option 2), some one else will be writing an issue soon enough complaining that they really NEED it to be fully opaque. so I'm inclined to make a more specific change like option 1 or 3. Once there is consensus, I'll make the PR along with the tutorial changes as @larsoner suggested. |
It's possible but really I think the transparent was probably always a better choice. We make aesthetic/functional judgments like this often for our plotting functions. This is why I wanted to make sure that, in your PR, you make some tiny change to each plot_alignment So I say we try (2) and look at the results. If they look bad, I'd prefer (3) to (1). Making it contingent on sEEG/ECoG does not solve the problem -- for example if you plot EEG+scalp and your dig point is off by a couple of mm it will be inside. In that case should the scalp be transparent? So now the transparency of the surface depends on the locations of the electrodes, etc. It's cleaner to have a standard behavior in all electrode + surface combinations if possible. |
@BarryLiu-97 by adding plot_alignment(raw.info, subject=subject, subjects_dir=subjects_dir,
trans=trans, show_axes=True, surfaces=["pial", "head"]) |
I agree with you. Why not set this para Customizable? So people can choose whether transparent or not? |
Thank you so much! |
I'm skeptical that option 2 will work, but I'm willing to be proven wrong.
I routinely use plot_alignment to check my coreg results by plotting scalp
surfaces and dig points. I know from various problems in mne_coreg that
being able to see the dig and FID points on both sides of the head through
the scalp surface is not helpful and is generally confusing. Hence my
concern.
Like i said I'm happy to be proven wrong but please consider use cases like
https://mne.tools/stable/auto_examples/visualization/plot_eeg_on_scalp.html#sphx-glr-auto-examples-visualization-plot-eeg-on-scalp-py
when evaluating the change.
Thanks
Luke
…On Wed, Oct 28, 2020 at 9:54 AM Keith Doelling ***@***.***> wrote:
@BarryLiu-97 <https://github.com/BarryLiu-97> by adding surfaces=["pial",
"head"] or just surfaces=["pial"] (for only brain and no skull) as a
parameter to plot_alignment should look like this
plot_alignment(raw.info, subject=subject, subjects_dir=subjects_dir,
trans=trans, show_axes=True, surfaces=["pial", "head"])
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#8445 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABKTXHOUH4CQYXVAYHAHGRTSNAPCPANCNFSM4TCHJWIA>
.
|
I'm thinking/hoping that this example will look better with So perhaps we should take a hybrid approach -- implement 2 and 3, so that in the (hopefully rare) cases where things do look worse, people have a way to get back to the old behavior. This is the approach we've been trying to take with Concretely @kdoelling1919 I would implement this by:
Does that sound like a reasonable plan? |
FYI @BarryLiu-97 the reason I was trying to avoid this is that every time you add more options for people it increases code complexity and maintenance burden at our end, and also makes it harder for people to understand (more options = more complicated interface). But I agree it's probably safer to allow it... |
I couldn't agree more. |
here is the output of mne coreg with 0.75 head opacity.. I don't think that additional information is helpful in this case. Which points are inside the head vs on the opposing side of the head? |
Yes I agree this looks confusing in this specific case. |
Yes I'm in agreement with @bloyl. Running these tutorials locally, I found that just making everything by default more transparent will make some other use cases harder to read. The reason I liked using only sEEG as a special case for transparency is because it's the only case we have (that I can think of) where you would purposefully have sensors inside the surface of interest, so I think it really is a special case that should be treated differently. Even in the scenario @larsoner mentioned, where one digitized point is accidentally inside the head, I'm not sure I would catch it from a plot like @bloyl just presented. |
I found a para seeg(bool) in plot_alignment, what if set alpha=0.75 when seeg=True? |
I think you get this because of your (unfortunately persistent) graphics problems. It looks better with a modern graphics setup / advanced rendering. But we still do block the points on the far side (that are not inside the head) intentionally in that case actually.
It turns out that hacking in a Becomes this: To me this is nicer, but probably still not good enough for sEEG. And adding depth peeling things become more challenging (and restrictive in terms of which graphics cards actually work). So I can live with leaving the default at 1.0 and just having
My proposal is to make it so you can do |
Fair enough. I think this line:
Is just a really convoluted |
I like the last figures @larsoner and @kdoelling1919 produced. |
This issue is fixed by #8446 |
Describe the new feature or enhancement
Using plot_alignment to see where sEEG electrodes are located in the brain. But if you you use any brain surfaces such as
surfaces=["pial"]
the brain is fully opaque so you can't see where the electrodes are that are inside the brain. So it would be best to give the brain a bit of transparency when you have sEEG electrodes embedded in the brain.Describe your proposed implementation
As far as I can tell, this is a single line change. I have already created a feature branch in my fork which changes the setting of opacity depending on whether there are sEEG electrodes present for plotting.
https://github.com/kdoelling1919/mne-python/blob/5b039cdfb3da64c89947f09a0bfd83481955f29c/mne/viz/_3d.py#L840
Describe possible alternatives
Another option might be to set transparency generally any time there are sensors to be plotted that are inside the brain volume. Sounds like a bit more work (and more knowledge about the surfaces than I currently have) but it would be more general.
Additional comments
Here is what the plot_alignment looks like in current master version of plot_alignment

Here is what it looks like in my branch.

If people agree this is useful beyond just for myself. I'll make the pull request.
Thank you!
The text was updated successfully, but these errors were encountered: