-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
BUG: Fix bug with scaling and fids for standard montages #10324
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| # Check a known location | ||
| assert_array_almost_equal(raw.info['chs'][0]['loc'][:3], | ||
| [0.0616, 0.075398, 0.07347]) | ||
| [0.054243, 0.081884, 0.054544]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@HanBnrd @rob-luke are the artinis montages added in #9141 in MNI / fsaverage space? If so, these new positions should be more accurate. Could you check if plot_alignment looks more accurate, or at least as accurate as before? The differences might be subtle (things will move slightly more anterior)...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @larsoner, yes artinis-brite23.elc and artinis-octamon.elc are using MNI coordinates. Sorry, what would you like to check exactly? Would you like us to run plot_alignment with fNIRS data for the Artinis montages and compare before and after your PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you like us to run plot_alignment with fNIRS data for the Artinis montages and compare before and after your PR?
Yeah that would be great. The changes suggest that they will shift anterior (and a little inferior) by ~1.5 cm but it should actually be more correct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @larsoner, this is before (left) and after (right) the PR with the Artinis Octamon montage


It seems that after the PR, the sensors look like they're inside the head in the 3D vis but this is a bit confusing because the 2D viz shows the sensors more anterior indeed. This is the code I used:
raw.set_montage('artinis-octamon')
raw.plot_sensors()
subjects_dir = mne.datasets.sample.data_path() + '/subjects'
mne.datasets.fetch_fsaverage(subjects_dir=subjects_dir, verbose=True)
fig = mne.viz.create_3d_figure(size=(800, 600), bgcolor='white')
fig = mne.viz.plot_alignment(raw.info, show_axes=True,
subject='fsaverage', coord_frame='mri',
trans='fsaverage', surfaces=['brain', 'head'],
fnirs=['channels', 'pairs',
'sources', 'detectors'],
dig=True, mri_fiducials=True,
subjects_dir=subjects_dir, fig=fig)
mne.viz.set_3d_view(figure=fig, azimuth=70, elevation=100, distance=0.4,
focalpoint=(0., -0.01, 0.02))There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that after the PR, the sensors look like they're inside the head in the 3D vis
It could be that the head surface that the Artinis folks used to create the montage was slightly different. This seems to be the case for our EEG standard montages at least. It's tough to know which one is "more correct" I guess...
| # TODO: This is actually quite big... | ||
| assert 15 < translation < 18 # mm | ||
| assert 0 < translation < 1 # mm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a sign that things work better (and that they were broken before by this amount)
|
thx @larsoner |
Closes #10321
The difference is subtle, but it's there:
And this last one is in better agreement with HOMER by eye (note the anterior shift in this PR).