iEEGCoordinateSystemDescription is not being written correctly

[adam2392]

Describe the bug

In my testing of #675 I was adding tests for iEEGCoordinateSystemDescription that we add to the coordsystem.json files. However, I noticed a bug that writes the description as 'n/a'.

Steps to reproduce

In test_write.py::test_coordsystem_json, one can comment out the lines asserting what the CoordinateSystemDescriptions for iEEG should be and you will see that the tests fail.

Expected results

In order to overcome this, perhaps it's best to briefly summarize how we dealt with iEEG coordinate frames. There exist coordinate frames represented in three different spaces: i) MNE, ii) filename and iii) BIDS.

For example, in MNE, we'll represent "FreeSurfer" coordsystem as 'fs_tal'. Since this isn't yet a supported coordinate system in BIDS, in BIDs, it would be represented as 'Other'. In the filename, then this would be encoded as space-fs (note that we get rid of the underscore else it wouldn't be BIDS-compliant). Tbh this is quite confusing and unfortunate, but no way I see around it rn.

Possible Solutions

This is probably a mne-bidsv0.8+ problem, since it doesn't really have a huge impact.

Possibly, a way for tackling this would be to refactor how coordsystem.json files are written, and keep track of a "MNE-coord-system" and "BIDS-coord-system" better, so they can be handled correctly in dig.py::_coordsystem_json.

Additional information

>>> sys_info()
Platform:      macOS-10.16-x86_64-i386-64bit
Python:        3.8.5 | packaged by conda-forge | (default, Jul 22 2020, 17:24:51)  [Clang 10.0.0 ]
Executable:    /Users/adam2392/opt/miniconda3/envs/mne/bin/python
CPU:           i386: 8 cores
Memory:        Unavailable (requires "psutil" package)
mne:           0.22.0
numpy:         1.19.1 {blas=NO_ATLAS_INFO, lapack=lapack}
scipy:         1.5.2
matplotlib:    3.3.0 {backend=TkAgg}

sklearn:       0.24.0
numba:         Not found
nibabel:       3.2.1
nilearn:       0.7.0
dipy:          Not found
cupy:          Not found
pandas:        1.1.3
mayavi:        Not found
pyvista:       0.26.1 {pyvistaqt=0.2.0, OpenGL 4.1 ATI-4.2.13 via AMD Radeon Pro 560 OpenGL Engine}
vtk:           9.0.1
PyQt5:         5.15.1

[sappelhoff]

or example, in MNE, we'll represent "FreeSurfer" coordsystem as 'fs_tal'. Since this isn't yet a supported coordinate system in BIDS,

Are you sure that it's not supported? Not even after the recent PRs to make any "standard coordinate system" acceptable? (see: https://bids-specification.readthedocs.io/en/latest/99-appendices/08-coordinate-systems.html#standard-template-identifiers)

In the filename, then this would be encoded as space-fs (note that we get rid of the underscore else it wouldn't be BIDS-compliant). Tbh this is quite confusing and unfortunate, but no way I see around it rn.

I actually think the filename should be space-Other, because the space should mirror what's in your CoordinateSystem field. But I know that the validator currently does not check that.

EDIT: found the corresponding validator issue: https://github.com/bids-standard/bids-validator/issues/743

[sappelhoff]

for posterity, these are the problematic lines:

mne-bids/mne_bids/tests/test_write.py

Lines 2178 to 2180 in d65b8ca

	TODO: Fix coordinatesystemdescription for iEEG.
	Currently, iEEG coordinate system descriptions are not written
	correctly.

mne-bids/mne_bids/tests/test_write.py

Lines 2204 to 2206 in d65b8ca

	# XXX: setting landmarks for ieeg montage for some reason
	# transforms coord_frame -> 'CapTrak'. Possible issue in mne
	landmarks = dict()

mne-bids/mne_bids/tests/test_write.py

Lines 2274 to 2286 in d65b8ca

	elif datatype == 'ieeg' and coord_frame == 'mni_tal':
	assert 'space-mni' in coordsystem_fname
	assert coordsystem_json['iEEGCoordinateSystem'] == 'Other'
	# assert coordsystem_json['iEEGCoordinateSystemDescription'] == \
	# COORD_FRAME_DESCRIPTIONS['mni_tal']
	elif datatype == 'ieeg' and coord_frame == 'fs_tal':
	assert 'space-fs' in coordsystem_fname
	assert coordsystem_json['iEEGCoordinateSystem'] == 'Other'
	# assert coordsystem_json['iEEGCoordinateSystemDescription'] == \
	# COORD_FRAME_DESCRIPTIONS['fs_tal']
	elif datatype == 'ieeg' and coord_frame == 'unknown':
	assert coordsystem_json['iEEGCoordinateSystem'] == 'Other'
	# assert coordsystem_json['iEEGCoordinateSystemDescription'] == 'n/a'

[sappelhoff]

The problem goes even deeper ... although we didn't have an issue with that so far, it's a ticking bomb: Look at these lines:

mne-bids/mne_bids/config.py

Lines 174 to 189 in d65b8ca

	BIDS_TO_MNE_FRAMES = {
	'ctf': 'ctf_head',
	'4dbti': 'ctf_head',
	'kityokogawa': 'ctf_head',
	'elektaneuromag': 'head',
	'chietiitab': 'head',
	'captrak': 'head',
	'acpc': 'ras',
	'mni': 'mni_tal',
	'fs': 'fs_tal',
	'ras': 'ras',
	'voxel': 'mri_voxels',
	'mri': 'mri',
	'unknown': 'unknown'
	}
	MNE_TO_BIDS_FRAMES = {val: key for key, val in BIDS_TO_MNE_FRAMES.items()}

Click details, which will show you that reversing a dict with duplicate values leads to data loss:

In [1]: BIDS_TO_MNE_FRAMES = {
   ...:     'ctf': 'ctf_head',
   ...:     '4dbti': 'ctf_head',
   ...:     'kityokogawa': 'ctf_head',
   ...:     'elektaneuromag': 'head',
   ...:     'chietiitab': 'head',
   ...:     'captrak': 'head',
   ...:     'acpc': 'ras',
   ...:     'mni': 'mni_tal',
   ...:     'fs': 'fs_tal',
   ...:     'ras': 'ras',
   ...:     'voxel': 'mri_voxels',
   ...:     'mri': 'mri',
   ...:     'unknown': 'unknown'
   ...: }
   ...: MNE_TO_BIDS_FRAMES = {val: key for key, val in BIDS_TO_MNE_FRAMES.items(
   ...: )}
   ...: 

In [2]: BIDS_TO_MNE_FRAMES
Out[2]: 
{'ctf': 'ctf_head',
 '4dbti': 'ctf_head',
 'kityokogawa': 'ctf_head',
 'elektaneuromag': 'head',
 'chietiitab': 'head',
 'captrak': 'head',
 'acpc': 'ras',
 'mni': 'mni_tal',
 'fs': 'fs_tal',
 'ras': 'ras',
 'voxel': 'mri_voxels',
 'mri': 'mri',
 'unknown': 'unknown'}

In [3]: MNE_TO_BIDS_FRAMES
Out[3]: 
{'ctf_head': 'kityokogawa',
 'head': 'captrak',
 'ras': 'ras',
 'mni_tal': 'mni',
 'fs_tal': 'fs',
 'mri_voxels': 'voxel',
 'mri': 'mri',
 'unknown': 'unknown'}

basically, having several values "head" means that upon reversal, only one of these values becomes a key - with its former key as a value. All other key-val pairs are getting dropped.