Mne-python: Digitization progress

[massich]

This issue is to keep track of the remaining Digitization PRs. It reflects a conversation with @larsoner.

TODO (each item will be a separate PR):

• [x] Add move all montage use to Raw and Epochs readers that have montage argument to call only the following at the end of their __init__, not used anywhere else (#6462):

  if montage is not None:
     self.set_montage(montage)
  

• [ ] Deprecate montage argument in all readers that support montage arguments (#6534)

  • [ ] Make we have a test like sure https://github.com/mne-tools/mne-python/issues/6461#issuecomment-503584614
    py def test_montage(): raw_montage = read_raw_*(fname, montage=my_montage) raw_none = read_raw_*(fname, montage=None) raw_none.set_montage(montage) assert object_diff(raw_none.info['chs'], raw_montage.info['chs']) assert object_diff(raw_none.info['dig'], raw_montage.info['dig'])

• [ ] ~Replicate montage and digmontage usecases with Digitization~ This no longer applies. We kept DigMontage, remove Montage and make digitization private.

  • [x] Refactor DigMontage to use Digitization + ch_names as representation (+ deprecate things that we no longer want to support) (#6639)
  • [x] Make digitization module private _digitization (#6700)
  • [ ] Fix DigMontage.save (saves dig but not ch_names)
  • [x] Fix DigMontage() without parmeters.
  • [ ] Refactor DigMontage tests. All that we did not change in #6639 because we only wanted to touch code logic not test logic.
  • [ ] set_montage takes only DigMontage not Montage
  • [ ] Some refactor of mne/_digitization. We have all _make_dig_{kit, artemist, bti} that overlap with DigMontage.
  • [ ] deprecate read_dig_montage in favor of read_dig_montage_FOO or make_dig_montage(np.arrays..). No transforms allowed.
  • [x] deprecate DigMontage.dev_head_t in favor of:
  • Storing the 2 versionsof the HPI points in the DigMontage.dig: one in FIFFV_COORD_HEAD and the other in FIFFV_COORD_MEG.
    When you want dev_head_t, call DigMontage.get_dev_head_t() and it will compute it from these directly.
  • a free function returning a proper transformation: def compute_dev_head_t(dig: Digitization) -> Transform:
  • [x] rename elp, hpi in favor of the new names (hpi, hpi_dev) in the internal calls. (I'm not sure this will be needed. We'll see)
  • [ ] DigMontage.transform_to_head() (not fully clear but it will get more clear when addressing the others). Things that could require:
  • a free function like: def get_fiducials(dig: Digitization) -> Fiducials: (Fiducials is a Bunch with nasion, lpa, rpa or similar, named tuple whatever.)
  • do all points need to be in 'head' or only some of them?
  • does it need to be a method? (probably yes)

• [ ] remove fit_match_points outside of coreg.

• [ ] move read_dig_XXXX to mne/io/

• [ ] ~Add plot() to Digitization class #6412. 3D plot with each dig point, maybe symbol based on coordinate frame (sphere=head, cube=meg, ... ?) and color based on type (eeg, extra, hpi, we have colors for these in defaults.py or plot_alignment somewhere)~ Fix the DigMontage-Montage mess we had in the existing plot with point_names and custom transforms. (#6649, maybe different PR)

• [ ] Make a none smoke test for applied transforms https://github.com/massich/mne-python/pull/31 (I've close it by error, it still applies)

---

Log of past PRs:

• Add example showing all the eeg helmets (#6158)
• MRG: Prepare BTI to refactor for Digitization (#6228)
• MAINT: Refactor digitation for KIT, artemis123, bti (#6368)
• ENH: Add digitization class (#6369)
• MNT: Change DigMontage representation to use Digitization (#6639)
• Make mne/digitization private mne/_digitization (#6700)

Cross-reference issues:

• https://github.com/mne-tools/mne-python/issues/3893#issuecomment-271925975
• #6698

---

Here is a summary of the readers with respect to how they are tested. Main focus: does the reader go through _test_raw_reader? has the reader been tested with digitization or None? Does the reader have digitization or a montage?

                       | _test_raw_reader | None | Digitization | other   |
-----------------------+------------------+------+--------------+---------+
RawArray               |     x            |  x   |              |         |
read_raw_artemis123    |     x            |      |     x        |         |   
read_raw_bdf           |                  |      |              | montage |
read_raw_brainvision   |     x            |  x   |     x        |         |    
read_raw_bti           |     x            |  x   |     x        |         |
read_raw_cnt           |     x            |  x   |              | montage |
read_raw_ctf           |     x            |      |     x        |         |
read_raw_edf           |     x            |  x   |     x        |         |
read_raw_eeglab        |     x            |      |     x        |         |
read_raw_egi           |     x            |  x   |              | montage |
read_raw_eximia        |     x            |  x   |              | ------- |
read_raw_fieldtrip     |                  |      |              | ------- |
read_raw_fif           |     x            |      |     x        |         |
read_raw_gdf           |     x            |  x   |              | montage |
read_raw_kit           |     x            |  x   |     x        |         |
read_raw_nicolet       |     x            |  x   |              | montage |

---

other notes:
1) duplicated montages

import os.path as op
import mne
fname = op.join(op.dirname(mne.io.edf.__file__), 'tests', 'data', 'biosemi.hpts')

default_biosemi64 = mne.channels.read_montage('biosemi64')
edf_testing_biosemi64 = mne.channels.read_montage(fname)

[GuillaumeFavelier]

Thanks for the overview @massich! I think there is also #6412 (add a plot() function to the Digitization class). I still have to integrate @agramfort reviews.

[massich]

@GuillaumeFavelier right!

[larsoner]

@massich updated top description with my suggested order / description based on extrapolating what I discussed with @agramfort

[massich]

just to make sure that we are all on the same page. This should pass without warning error nothing:

def test_montage():
    raw_montage = read_raw_*(fname, montage=my_montage)
    raw_none = read_raw_*(fname, montage=None)
    raw_none.set_montage(montage)
    assert object_diff(raw_none.info['chs'], raw_montage.info['chs'])
    assert object_diff(raw_none.info['dig'], raw_montage.info['dig'])

[agramfort]

yes exactly

[larsoner]

Yes in this PR. Eventually in this release cycle we will deprecade the read_raw_*(fname, montage=my_montage), but not in this PR. (It should be almost trivial after this PR.)

[larsoner]

Sorry, I meant to say that yes, that should be the behavior in #6462.

In the PR after that, read_raw_*(fname, montage=my_montage) should be deprecated and the user told to do raw.set_montage(my_montage).

[massich]

update_ch_names has been exposed in 0.19. Once Digitization is here, we should be able to remove it from the signature.
https://github.com/mne-tools/mne-python/blob/2f9873e7759e62f8471326e37a2ad1cdd2c2e65d/mne/channels/channels.py#L446

In other words, Digitization should go in before releasing so that we don't need to deprecate this parameter.

[massich]

now that montage is deprecated, the idea is that raw.set_montage only takes Digitization and that as a middle step montages and digmontages had to be transformed to Digitizations, am I right? @agramfort, @larsoner

[larsoner]

Yeah I think so.

[agramfort]

question is should we still call it set_montage or we introduce a new
method set_digitization?
just thinking out loud...

[massich]

set_montage sounds fine to me

[massich]

There are at least two ways of getting the digitization information from a fif file:

https://github.com/mne-tools/mne-python/blob/897c3b2d12d0562f888784c5e18c2da31ed27ceb/mne/io/meas_info.py#L891

https://github.com/mne-tools/mne-python/blob/897c3b2d12d0562f888784c5e18c2da31ed27ceb/mne/channels/montage.py#L637

it is true that one populates meas_info while the other returns a DigMontage, but they are essentially the same.

(The later calls the former)

[agramfort]

not really as the anything is done after _read_dig_fif(fid, tree) is
just to make a DigMontage from a dig (get numpy arrays and a few attributes)

>

[massich]

Regarding the kind we have this FIF.FIFV_MNE_COORD_DIGITIZER and FIFFV_COORD_ISOTRAK that is used nowhere.

~/code/mne-python read_dig_montage_polhemus
(mne) ❯ git grep -i 'FIFFV_MNE_COORD_DIGITIZER'
mne/io/constants.py:FIFF.FIFFV_MNE_COORD_DIGITIZER   = FIFF.FIFFV_COORD_ISOTRAK # Original (Polhemus) digitizer coordinates
mne/io/tests/test_constants.py:    FIFFV_MNE_COORD_DIGITIZER='FIFFV_COORD_ISOTRAK',

[agramfort]

yes then we need to change the read_dig_xxx so they don't use unknown but
the proper
source of the data. We would need to add a constant for captrack though

[teonbrooks]

explaining some context behind the KIT digitization process at NYU based on a discussion with @massich.

so for the KIT system, the use of the labels elp and hpi had come from the naming used in the old mne-c manual.

HPI
Head Position Indicator (HPI) points come from the head localization procedure in the MEG with the attached coils. There are five coils for the KIT system, these points have a specific order, and that this allows for the coregistration of the head into MEG device space for the dev_head_t computation. This is stored in either a .mrk file or a .sqd file, which just contains this head localization procedure.

ELP
The electrode points file has been constructed to emulate the .elp file structure that was produced from the old ISOTRAK pen IIRC. In this file, there is a section for the fiducials which exists at the top, and there is a section for the corresponding HPI points but collected in the digitizer native space.
The NYU lab with the KIT system has been accustomed to include all 8 points in an exported txt file from the FASKTRAK digitizer. This has been done as a matter of lab protocol convention.

I think that ideally there should be three files as opposed to two files: one for the HPI points (5 points), one for those points in digitizer space (5 points), and one file for the fiducials (3 points). I don't think including the fiducials in the points file is a good idea anymore because this requires an arbitrary assignment of points to references. From my discussion with @massich, it is true that someone could place the points in any manner they sought fit but there is no way for the software to intelligently know this.

File Format
FASKTRAK allows for exports to txt files but there isn't anything inherently special about it, it just arbitrary number of point x 3. These points are referenced to a reference box therefore all the digitizer data must be re-reference to neuromag head space based on fiducials. The polhemus file reader is just one that ignores the header and just reads the data in as an array.

Naming
As for the names of HPI and ELP, there has been a proposal to rename the points:
HPI --> hpi_dev
ELP --> hpi

After thinking about this, I think that HPI should remain being hpi and ELP should be the one renamed to something like hpi_isotrak or hpi_isotrak_space
HPI --> hpi
ELP --> hpi_isotrak

hope this provides some context and recommendations.

[larsoner]

@massich do we need any of the rest of these for 0.19? Or just #6764?

[massich]

I think #6764 is enough. I'm closing this one