Skip to content

Conversation

@GuillaumeFavelier
Copy link
Contributor

@GuillaumeFavelier GuillaumeFavelier commented Jul 1, 2021

This PR refactors some traits-dependant classes out of GUI code. The goal is to be able to perform coregistration without the need for an interface.

Getting rid of traits means reproducing the dependency relations between properties/methods. It's still a draft.
It's an item of #8833

@GuillaumeFavelier GuillaumeFavelier self-assigned this Jul 1, 2021
@GuillaumeFavelier
Copy link
Contributor Author

Here is the modified version of jhouck's work (that I use as baseline) using the new Coregistration class:

For now, data_path is still used but the goal is to require info instead. I'll modify that next.

Among the more pressing issues, I have to continue refactor the class to remove the following from gui somehow:

class Coregistration(object):
    def __init__(self, data_path, subject, subjects_dir):
        from .gui._file_traits import DigSource
        from .gui._fiducials_gui import MRIHeadWithFiducialsModel
...
"""
=========================================
Use automated approach to co-registration
=========================================
This example shows how to use the get_mni_fiducials routine and the
coregistration GUI functions to perform an automated MEG-MRI co-registration.
.. warning:: The quality of the co-registration depends heavily upon the
             quality of the head shape collected during subject prepration and
             the quality of your T1-weighted MRI. Use with caution and check
             the co-registration error.
"""

# License: BSD (3-clause)

import os.path as op
import numpy as np
import mne
from mne.coreg import get_mni_fiducials, Coregistration
from mne.surface import dig_mri_distances
from mne.io import write_fiducials
from mne.io.constants import FIFF

data_path = mne.datasets.sample.data_path()
subjects_dir = op.join(data_path, 'subjects')
subject = 'sample'
fname_raw = op.join(data_path, 'MEG', subject, subject + '_audvis_raw.fif')
fname_fids = op.join(subjects_dir, subject, 'bem', subject + '-fiducials.fif')
fname_trans = op.join(data_path, 'MEG', subject,
                      subject + 'audvis_raw_auto-trans.fif')
src = mne.read_source_spaces(op.join(subjects_dir, subject, 'bem',
                                     'sample-oct-6-src.fif'))

fids_mri = get_mni_fiducials(subject, subjects_dir)

# Save mri fiducials. This is mandatory. as fit_fiducials uses this file

write_fiducials(fname_fids, fids_mri, coord_frame=FIFF.FIFFV_COORD_MRI)
print('\nSaving estimated fiducials to %s \n' % fname_fids)

coreg = Coregistration(data_path, subject, subjects_dir)
coreg.fit_fiducials()
coreg.fit_icp(iterations=6, nasion_weight=2.)
coreg.omit_hsp_points(distance=5. / 1000)
coreg.fit_icp(iterations=20, nasion_weight=10.)
coreg.save_trans(fname=fname_trans)
errs_icp = coreg.point_distance()

raw = mne.io.Raw(fname_raw)
errs_nearest = dig_mri_distances(raw.info, fname_trans, subject, subjects_dir)

fig = mne.viz.plot_alignment(raw.info, trans=fname_trans, subject=subject,
                             subjects_dir=subjects_dir, surfaces='head-dense',
                             dig=True, eeg=[], meg='sensors',
                             coord_frame='meg')
mne.viz.set_3d_view(fig, 45, 90, distance=0.6, focalpoint=(0., 0., 0.))

print('Median distance from digitized points to head surface is %.3f mm'
      % np.median(errs_icp * 1000))
print('''Median distance from digitized points to head surface using nearest
neighbor is %.3f mm''' % np.median(errs_nearest * 1000))

@GuillaumeFavelier
Copy link
Contributor Author

The latest version of the testing example using info:

"""
=========================================
Use automated approach to co-registration
=========================================
This example shows how to use the get_mni_fiducials routine and the
coregistration GUI functions to perform an automated MEG-MRI co-registration.
.. warning:: The quality of the co-registration depends heavily upon the
             quality of the head shape collected during subject prepration and
             the quality of your T1-weighted MRI. Use with caution and check
             the co-registration error.
"""

# License: BSD (3-clause)

import os.path as op
import numpy as np
import mne
from mne.coreg import get_mni_fiducials, Coregistration
from mne.surface import dig_mri_distances
from mne.io import write_fiducials, read_info
from mne.io.constants import FIFF

data_path = mne.datasets.sample.data_path()
subjects_dir = op.join(data_path, 'subjects')
subject = 'sample'
fname_raw = op.join(data_path, 'MEG', subject, subject + '_audvis_raw.fif')
fname_fids = op.join(subjects_dir, subject, 'bem', subject + '-fiducials.fif')
fname_trans = op.join(data_path, 'MEG', subject,
                      subject + 'audvis_raw_auto-trans.fif')
src = mne.read_source_spaces(op.join(subjects_dir, subject, 'bem',
                                     'sample-oct-6-src.fif'))

fids_mri = get_mni_fiducials(subject, subjects_dir)

# Save mri fiducials. This is mandatory. as fit_fiducials uses this file

write_fiducials(fname_fids, fids_mri, coord_frame=FIFF.FIFFV_COORD_MRI)
print('\nSaving estimated fiducials to %s \n' % fname_fids)

info = read_info(fname_raw)
coreg = Coregistration(info, subject, subjects_dir)
coreg.fit_fiducials()
coreg.fit_icp(iterations=6, nasion_weight=2.)
coreg.omit_hsp_points(distance=5. / 1000)
coreg.fit_icp(iterations=20, nasion_weight=10.)
coreg.save_trans(fname=fname_trans)
errs_icp = coreg.point_distance()

raw = mne.io.Raw(fname_raw)
errs_nearest = dig_mri_distances(raw.info, fname_trans, subject, subjects_dir)

fig = mne.viz.plot_alignment(raw.info, trans=fname_trans, subject=subject,
                             subjects_dir=subjects_dir,
                             surfaces=dict(head=0.4),
                             dig=True, eeg=[], meg=False,
                             coord_frame='meg')
mne.viz.set_3d_view(fig, 45, 90, distance=0.6, focalpoint=(0., 0., 0.))

print('Median distance from digitized points to head surface is %.3f mm'
      % np.median(errs_icp * 1000))
print('''Median distance from digitized points to head surface using nearest
neighbor is %.3f mm''' % np.median(errs_nearest * 1000))

@GuillaumeFavelier GuillaumeFavelier marked this pull request as ready for review July 12, 2021 15:42
mne/coreg.py Outdated
self.eeg_weight = 1.
self.hpi_weight = 1.

self.hsp = _DigSource(info)
Copy link
Member

Choose a reason for hiding this comment

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

we should consider renaming "hsp" as it's really jargon.

Copy link
Member

Choose a reason for hiding this comment

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

It's shorthand for "headshape points" and a holdover of Polhemus systems. I'm fine with standardizing around something else like "extra points" (which is what we call it in info/dig) but make sure that HSP here does not include other stuff (eeg points, HPI points, etc.) and really just includes these "extra" dig points

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know. It seems to be the case:

  27     @property
  26     def _transformed_hsp_hpi(self):
  25         return apply_trans(self._hsp_trans, self._hsp.hpi_points)
  24 
  23     @property
  22     def _transformed_hsp_eeg_points(self):
  21         return apply_trans(self._hsp_trans, self._hsp.eeg_points)
  20 
  19     @property
  18     def _transformed_hsp_points(self):
  17         return apply_trans(self._hsp_trans, self._hsp.points)

@agramfort
Copy link
Member

did you add a test for the fids parameter?

it's close to ready from my end. Maybe @larsoner wants to have a look before we merge.

I think you can safely start drafting the GUI on top of this class now @GuillaumeFavelier

@jhouck
Copy link
Contributor

jhouck commented Aug 9, 2021

@jhouck can you also suggest some wording improvements in the example? Can you also test this branch now to confirm it works as it should?

We’re getting there !

@agramfort sure! I'm out of the office until Friday and can have a look then, if that's not too late.

@agramfort agramfort changed the title WIP,ENH: Refactor coreg/traits classes MRG,ENH: Refactor coreg/traits classes Aug 17, 2021
@agramfort
Copy link
Member

I think it is now good for review from my end.

@GuillaumeFavelier see my bug fixes in setup_fiducials. You were actually always reading
fiducials from disk and the dict path was broken. I also now allow to pass directly the output
of read_fiducials function.

@larsoner you have time for review?

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

LGTM after a refactoring, @GuillaumeFavelier just make sure you're happy with e13925e

@agramfort
Copy link
Member

example looks good https://32845-1301584-gh.circle-artifacts.com/0/dev/auto_tutorials/forward/25_automated_coreg.html but the Coregistration class is not clickable. I'll look into this.

@agramfort
Copy link
Member

@larsoner
Copy link
Member

It's a weird bug where having something documented at the level of mne.coreg.Coregistration but available at a higher level mne.Coregistration confused sphinx-gallery. I'll remove it from mne/__init__.py since it seems like it was meant to live just in the mne.coreg namespace (and I agree it's nicer than having it at the root level)

@agramfort agramfort merged commit f156630 into mne-tools:main Aug 18, 2021
@agramfort
Copy link
Member

thx @GuillaumeFavelier !!!

@agramfort agramfort mentioned this pull request Aug 18, 2021
@GuillaumeFavelier GuillaumeFavelier deleted the mnt/traits branch August 23, 2021 07:53
@GuillaumeFavelier GuillaumeFavelier restored the mnt/traits branch August 23, 2021 08:04
@GuillaumeFavelier GuillaumeFavelier deleted the mnt/traits branch August 23, 2021 08:04
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