[ENH, MRG] Add sensor projection for ECoG

[alexrockhill]

Add sensor projection to the brain surface to account for brain shift.

[alexrockhill]

This looks okay to me besides that there is a split across the gyrus in the larger array https://35145-1301584-gh.circle-artifacts.com/0/dev/auto_tutorials/clinical/10_ieeg_localize.html

Any ideas on how to fix that?

[larsoner]

My only idea on the gyral jump is that the brain surface might have some bump/irregularity causing a problem. Have you plotted by eye to see if it makes sense? Some plot_alignment call with surfaces=dict(brain=1.0) should make it clear I think. If it's wrong, sometimes you can fix it by tweaking the FreeSurfer calls in the right places...

If we can't fix it, maybe just make a note that it's probably incorrect for these data but it's the best that we can do? Keep in mind that we sort of know FreeSurfer doesn't make perfect BEM surfaces at this point (though they're usually "good enough" for M/EEG), see for example #8840, #8825, and a host of other similar issues over the years.

From an API standpoint I wonder if this should live in a namespace like:

mne.preprocessing.ieeg.project_sensors_onto_brain

similar to how we have fNIRS-specific stuff in

mne.preprocessing.nirs.beer_lambert_law

It helps make it clear that this is really a data cleaning/fixing step, and that it's really only applicable to iEEG data. (At the end of the day we need to put less stuff in mne. rather than more when possible... it's already too crowded in there.)

[alexrockhill]

My only idea on the gyral jump is that the brain surface might have some bump/irregularity causing a problem. Have you plotted by eye to see if it makes sense? Some plot_alignment call with surfaces=dict(brain=1.0) should make it clear I think. If it's wrong, sometimes you can fix it by tweaking the FreeSurfer calls in the right places...

If we can't fix it, maybe just make a note that it's probably incorrect for these data but it's the best that we can do? Keep in mind that we sort of know FreeSurfer doesn't make perfect BEM surfaces at this point (though they're usually "good enough" for M/EEG), see for example #8840, #8825, and a host of other similar issues over the years.

It doesn't look like the brain surface is the issue...

From an API standpoint I wonder if this should live in a namespace like:

mne.preprocessing.ieeg.project_sensors_onto_brain

similar to how we have fNIRS-specific stuff in

mne.preprocessing.nirs.beer_lambert_law

It helps make it clear that this is really a data cleaning/fixing step, and that it's really only applicable to iEEG data. (At the end of the day we need to put less stuff in mne. rather than more when possible... it's already too crowded in there.)

I wrote it so that it could work for any sensors to any surface so that it would be more general in case there was a use case in the future. I think a refactoring of ieeg or intracranial-specific code including what I've added recently that uses marching cubes would be good but I think that would be better as a separate PR like the _freesurfer.py one.

[larsoner]

I think a refactoring of ieeg or intracranial-specific code including what I've added recently that uses marching cubes would be good but I think that would be better as a separate PR like the _freesurfer.py one.

Except that this adds a new public API, so we should decide the right place for it to live before merge, rather than after

[larsoner]

It doesn't look like the brain surface is the issue...

Doesn't the brain surface come inside the pial surface in many places? That doesn't look right to me -- it should be strictly outside the pial brain surafce if it's correctly defined.

[larsoner]

Ooof especially that right temporal pole pial surface is way outside the brain surface

[alexrockhill]

I think a refactoring of ieeg or intracranial-specific code including what I've added recently that uses marching cubes would be good but I think that would be better as a separate PR like the _freesurfer.py one.

Except that this adds a new public API, so we should decide the right place for it to live before merge, rather than after

This is the very last public API in the intracranial steps, everything else is done, I just was focusing on SEEG but this is good to add for completeness. The last one seems like maybe not the best time for major refactoring...

[alexrockhill]

It doesn't look like the brain surface is the issue...

Doesn't the brain surface come inside the pial surface in many places? That doesn't look right to me -- it should be strictly outside the pial brain surafce if it's correctly defined.

I'm not sure this is the source of the problem, I thought it was because the center of the grid was farther away from the surface than the edges leading to a bifurcation point when it's closer to one half than the other, not sure though

[larsoner]

The last one seems like maybe not the best time for major refactoring...

I'm not really talking about any major refactoring regarding the public API, just where the newly added (in this PR) project_sensors_onto_brain should live. I think other public functions you're recently added for iEEG live in a reasonable place -- the iEEG GUI is in mne.gui (reasonable) and the marching cubes is private now (mne.surface._marching_cubes). Do you have ideas for other public functions that should be moved before we cut 0.24 in about a month? As it stands, I'm -1 on merging this because mne.project_sensors_onto_brain might not be the best place for it to live in perpituity.

Separately, I mentioned that the internal implementation of mne.stc_near_sensors should be refactored with the project_sensors_onto_brain code newly added in this PR. This PR adds code that is not DRY with the existingo one, so that needs to be addressed in this PR. We don't want to get in the habit of making PRs that share a lot of functionality with other existing code -- we need to promote code reuse and refactoring internally (it's good coding practice). Hence I'm -1 on merging this one as is until it's refactored with with stc_near_sensors, as well.

Does these two ideas make sense?

[alexrockhill]

That does seem to be the issue (i.e. the brain surface is okay) because the projection onto inflated looks like this

It looks like there are two papers cited by iELVis on how to deal with the issue but their implementation is a bit complex https://www.biorxiv.org/content/10.1101/069179v2.full...

[larsoner]

That does seem to be the issue (i.e. the brain surface is okay)

I'm not sure what you're plotting, except that the green dots are outside not just the skull but even the scalp (esp. for the parietal/occipital electrodes). To me this looks like a head<->MRI problem, but it could be something else...

[larsoner]

because the projection onto inflated looks like this

Okay you updated -- you cannot project onto inflated and expect it to make sense, its geometry should be thought of as (mostly) independent of that of the realistic pial/white/brain/skull/scalp surfaces. It basically shares a triangulation with pial/white, but the positions don't make sense. IIRC it even is centered at the origin, which clearly is nonsensical for a hemisphere. (There is some geometric gymnastics done in Brain to deal with this fact IIRC.)

[alexrockhill]

Okay well it seems like this simple projection doesn't really do the job and my best guess is because of the geometry. The way it has been solved before involves considering each device separately which would need to be integrated into the GUI because that's where the device groups live. I'm not sure if there is a better, more general solution but it looks since there have been two papers on it, it's not as simple as it looks.

[alexrockhill]

Hmmm I tried the method of projecting using the normal vector defined by the two nearest neighbors and it sort of works better maybe it can be cleaned up...

[alexrockhill]

Alright, this looks pretty good to me! Pushing a commit with the code shortly. I put it in mne.preprocessing.ieeg but now it doesn't really refactor with stc_near_sensors because it doesn't use the projection code at all.

[alexrockhill]

Ok looks pretty good, ready for review

https://35184-1301584-gh.circle-artifacts.com/0/dev/auto_tutorials/clinical/10_ieeg_localize.html

[agramfort]

just a nitpick

please merge if CIs are green

[larsoner]

Feel free to add Passing a schema to Validator around https://github.com/mne-tools/mne-python/blob/main/mne/conftest.py#L127 to get the CIs happy if you want!

[larsoner]

Thanks @alexrockhill !