Skip to content

[ENH] Clarify SEEG tutorial coordinate frames#9601

Merged
larsoner merged 4 commits intomne-tools:mainfrom
alexrockhill:tut2
Jul 22, 2021
Merged

[ENH] Clarify SEEG tutorial coordinate frames#9601
larsoner merged 4 commits intomne-tools:mainfrom
alexrockhill:tut2

Conversation

@alexrockhill
Copy link
Contributor

This clarifies and I think simplifies what's going on with the coordinate transforms for the SEEG case.

I think this will be really helpful for the GUI as they will hopefully be able to follow the same steps.

@alexrockhill
Copy link
Contributor Author

I need to add a test for the new method, doing that now

@alexrockhill
Copy link
Contributor Author

Not to toot my own horn but I really like the applying a trans to a montage. I think it's very clean because the montage checks the coordinate frames for you, you just have to operate at the abstract level of coordinate frames, and it solves this issue that all info['dig'] is stored in the "head" coordinate frame by just requiring a trans file and then the montage can be got and then transformed in just a few lines.

I think this works really well for finding channel coordinates from just a CT; the fiducials could be found on the CT and a head to CT transform could be used to get the coordinates in CT space and be applied directly to the montage.

Copy link
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really nice !

@drammock
Copy link
Member

changes/latest.inc:118: WARNING: py:meth reference target not found: mne.channels.montage.apply_trans

you need to add it to doc/python_reference.rst (or rather, one of the other .rst files in that directory that it includes)

Copy link
Member

@drammock drammock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having trouble following 10_ieeg_localize.html, although the places that I find difficult to follow (or where I find typos/awkward passages) appear to have not changed in this PR, so maybe this is not the best place to discuss fixes. @alexrockhill LMK if you would prefer to make those changes in this PR, and if so, we can schedule a time to talk it through.

@alexrockhill
Copy link
Contributor Author

I'm having trouble following 10_ieeg_localize.html, although the places that I find difficult to follow (or where I find typos/awkward passages) appear to have not changed in this PR, so maybe this is not the best place to discuss fixes. @alexrockhill LMK if you would prefer to make those changes in this PR, and if so, we can schedule a time to talk it through.

Sure that's fine, can you talk this morning? Feel free to coordinate on Discord

@larsoner
Copy link
Member

From discord with @alexrockhill and @drammock we decided to merge this and @alexrockhill will 1) make a PR to clarify the narrative after the GUI work is done (or as part of it) and 2) open a dipy issue to see if SDR provides a direct (non-discretized) mapping from points in the moving space to the static space (i.e., the equivalent of an affine np.dot(affine, np.r_[pt, 1.])[:3])

@larsoner larsoner merged commit 64d51d6 into mne-tools:main Jul 22, 2021
@larsoner
Copy link
Member

Thanks @alexrockhill !

@alexrockhill alexrockhill deleted the tut2 branch July 22, 2021 17:50
# Freesurfer surfaces when they were assigned to the ``raw`` object, they were
# transformed to "head" so we need to have a transform to put them back
subj_trans = mne.read_trans(op.join(misc_path, 'seeg',
'sample_seeg_trans.fif'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry I saw this late. Is it kosher to have sEEG data as fif files and thus be transformed to "head" coordinates? Don't you normally get the coordinates in MRI coordinates as a separate text file or mat file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I agree it's not ideal but I think to be backward compatible with MEG/EEG, FIF requires it to be in the "head" frame. I've thought it a bit like how Freesurfer uses surface RAS as its internal representation frame, MNE uses "head". They are all arbitrary reference frames and at least there is a standard and you know how it's stored. Ideally the coordinate frame could be specified in the FIF but that seems like pretty large change.

marsipu pushed a commit to marsipu/mne-python that referenced this pull request Aug 2, 2021
* rework transform narrative

* add more clarifying language

* add test

* Update doc/changes/latest.inc

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants