Skip to content
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

MRG: Store NIRx location data in meters #6856

Merged
merged 4 commits into from
Oct 2, 2019

Conversation

rob-luke
Copy link
Member

@rob-luke rob-luke commented Oct 1, 2019

What does this implement/fix?

Stores the NIRx sensor location data in meters (as specified for 3d locations here)

Details

Note

The values displayed in the vendor software are in mm. However, the values stored with the recordings are in cm (I have confirmed this with the company technical support). And MNE expects m.

@codecov
Copy link

codecov bot commented Oct 1, 2019

Codecov Report

Merging #6856 into master will increase coverage by 1.33%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #6856      +/-   ##
==========================================
+ Coverage   88.28%   89.61%   +1.33%     
==========================================
  Files         426      425       -1     
  Lines       76034    76018      -16     
  Branches    12402    12405       +3     
==========================================
+ Hits        67123    68125    +1002     
+ Misses       6135     5116    -1019     
- Partials     2776     2777       +1

@larsoner
Copy link
Member

larsoner commented Oct 1, 2019

@rob-luke this code:

import mne
data_path = mne.datasets.testing.data_path()
subjects_dir = mne.datasets.sample.data_path() + '/subjects'
raw = mne.io.read_raw_nirx(data_path + '/NIRx/nirx_15_2_recording_w_short')
mne.viz.plot_alignment(
    raw.info, show_axes=True, subject='fsaverage', trans='fsaverage',
    subjects_dir=subjects_dir, verbose=True)

Now produces:
snapshot

You can see that the points are on the head (good) and that the head and MRI coordinate frames are correct (good). There are a few points that are quite far away to the subject's right, though -- is this correct/expected?

@rob-luke
Copy link
Member Author

rob-luke commented Oct 1, 2019

Thanks @larsoner! I loaded a bunch of other montages and they all look great.

I investigated that point that is slightly off the head by looking at other montages I have with more channels in that same area. All the other points sit nicely on the head. The one that is off the head looks to be over a recessed section near the ear. I have tried to capture this in the image below (look at the red dot on the left of the image ). It could be that because the channel is defined as the half way point between a source and detector, that it can then float in space if their is a recess in the head.

image

So I think this looks great and will change the title to MRG. I am looking forward to uploading some more interesting montages to make testing things like this easier.

@rob-luke rob-luke changed the title WIP: Store NIRx location data in meters MRG: Store NIRx location data in meters Oct 1, 2019
@agramfort
Copy link
Member

@rob-luke you need to fix merge conflict

@rob-luke
Copy link
Member Author

rob-luke commented Oct 2, 2019

I have uploaded another test dataset here.: mne-tools/mne-testing-data#52

And running @larsoner code above on this new data I get this image, which matches well with the vendor images

image

@larsoner larsoner merged commit 03b58ad into mne-tools:master Oct 2, 2019
@larsoner
Copy link
Member

larsoner commented Oct 2, 2019

Thanks @rob-luke, feel free to add tests with the new dataset (as necessary) in the next PR

alexrockhill pushed a commit to alexrockhill/mne-python that referenced this pull request Oct 3, 2019
* Change stored nirx locations to meters

* STY: Compact tests

* FIX: Fix coordinate frames and plot_alignment for fnirs
@rob-luke rob-luke deleted the nirx-fix-location-units branch April 8, 2022 05:26
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.

None yet

3 participants