Nihon Kohden EEG file reader WIP

[fraimondo]

This is a reader module for Nihon Kohden native EEG file format.

It is a WIP, so for now it does not read events, no metadata and the channel names are the standard ones (it can be renamed through a file).

I still need to add a test.

I leave it here so we can start doing the collaborative stuff and if anyone has any interest, they can chip in.

[larsoner]

This pull request introduces 1 alert when merging 4b922d8 into ce50c2b - view on LGTM.com

new alerts:

• 1 for Unused local variable

---

Comment posted by LGTM.com

[larsoner]

@fraimondo can you get some tiny file in mne-testing-data? I don't have any Nihon Koden data, but once that testing file is there, I (and others) can help with this.

[fraimondo]

I need to check who has the system that can provide me with a testing dataset. I’ll come back soon.

…

On 27 Mar 2019, at 20:11, Eric Larson ***@***.***> wrote: @fraimondo <https://github.com/fraimondo> can you get some tiny file in mne-testing-data? I don't have any Nihon Koden data, but once that testing file is there, I (and others) can help with this. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6017 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AESRg0sW45D_IRbW0hsA_8dHkL48ApZeks5va8JPgaJpZM4bamKI>.

[fraimondo]

Just created a PR in mne-testing-data. edit: see mne-tools/mne-testing-data#46

…

On 2 Apr 2019, at 14:39, Fede Raimondo ***@***.***> wrote: I need to check who has the system that can provide me with a testing dataset. I’ll come back soon. > On 27 Mar 2019, at 20:11, Eric Larson ***@***.*** ***@***.***>> wrote: > > @fraimondo <https://github.com/fraimondo> can you get some tiny file in mne-testing-data? I don't have any Nihon Koden data, but once that testing file is there, I (and others) can help with this. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub <#6017 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AESRg0sW45D_IRbW0hsA_8dHkL48ApZeks5va8JPgaJpZM4bamKI>. >

[larsoner]

@fraimondo any time to come back to this in the next couple of weeks to make it into 0.21? You now have your testing file in mne-testing-data.

If not, okay for me to push some commits to get tests working?

[fraimondo]

Just updated the code. This works. Can be improved by reading the annotations and more metadata.

[fraimondo]

There's an issue with the channel names. Hold the PR.

[larsoner]

Just updated the code. This works. Can be improved by reading the annotations and more metadata.

Awesome, I was just about to look into this to see if we could get into usable shape in the next week to make it into 0.21. Let me know if you need help

[fraimondo]

Can't figure out this one. I'm getting None as the cals on the _read_segment_file function. I guess it's related to this commit: 06f232c

So now I use the _mult_cal_one function. But it still fails. Apparently the length of the idx parameter does not match data.

It gives me an error that it can't broadcast the data after _mult_cal_one. So here's the issue:

> /Users/fraimondo/dev/mne-python/mne/io/nihon/nihon.py(394)_read_segment_file()
    392         import pdb; pdb.set_trace()
    393         # block_data = ((block_data[idx, :] * cals + offsets) * gains)
--> 394         _mult_cal_one(data, block_data, idx, cals, mult)
    395         data[:, :] += offsets
    396         data[:, :] *= gains

ipdb> data.shape                                                                                                                        
(1, 5800)
ipdb> idx.shape                                                                                                                         
(24,)

any clue?

The problem with this kind of files is that the data needs to be multiplied by the cals, then add offset and finally multiply by units (gains).

[larsoner]

I recently made some changes to this function. Okay for me to look and push a commit tomorrow?

[fraimondo]

Feel free. PR is ready to be merged one this is solved. The only solution I have in mind is to set cals=1 and then do the `(*scaling + offset) * gains` before calling `_mult_cal_one`.

…

On 7 Sep 2020, at 14:39, Eric Larson ***@***.***> wrote: I recently made some changes to this function. Okay for me to look and push a commit tomorrow? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6017 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABCJDAZ6MFWBPX6BKX54MIDSETIBJANCNFSM4G3KMKEA>.

[larsoner]

Setting cals=1 should be fine. The edf reader was updated to behave this way.

Basically the second arg of _mult_cal_one (one) should always have shape (n_data_channels, n_times) (and n_times can be shorter if you use it in chunks / with time-based slicing) then the _mult_cal_one will subindex the idx from n_data_channels that it needs. It's a bit wacky but it should work. The idx is really there just to tell you which rows of the one you actually need to fill / will be used.

[fraimondo]

Yes, I got that. But when a projector is applied while picking channels (as in the `_test_raw_reader` function), the `mult` parameter has shape (n_proj, n_channels). So the way I was offsetting and multiplying by units does not work, since `data` will have shape (n_proj, n_times). I’ll do the cal=1 trick.

…

On 7 Sep 2020, at 14:45, Eric Larson ***@***.***> wrote: Setting cals=1 should be fine. The edf reader was updated to behave this way. Basically the second arg of _mult_cal_one (one) should always have shape (n_data_channels, n_times) (and n_times can be shorter if you use it in chunks / with time-based slicing) then the _mult_cal_one will subindex the idx from n_data_channels that it needs. It's a bit wacky but it should work. The idx is really there just to tell you which rows of the one you actually need to fill / will be used. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6017 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABCJDA5AL43WJ7PB4ZUIUGTSETIVBANCNFSM4G3KMKEA>.

[larsoner]

But when a projector is applied while picking channels (as in the _test_raw_reader function), the mult parameter has shape (n_proj, n_channels). So the way I was offsetting and multiplying by units does not work, since data will have shape (n_proj, n_times).

You should not need to worry about the shape of mult EDIT: or the shape of data. If you make sure one has shape (n_channels, n_times), internally _mult_cal_one will subselect the correct channels after projection (this actually happens just by mult @ one, more or less). So your job as the reader-writer is to make sure one is of shape (n_channels, n_times) with the idx indices populated. Then everything should work. You shouldn't ever use cals yourself, and you should make sure that you know that they will be multiplied into the data values. So if your pattern is one = (one * a + b) * c as above, you can either set cals to 1. for all channels in info['ch'][ii]['cal'], or you can set them to c in the expression above and only take care of the one = (one * a + b) part of the expression (and let _mult_cal_one do the cals mult for you). What you don't want is it happening in both places.

[agramfort]

CIs complain @fraimondo

maybe rebase on master to fix it?

[fraimondo]

Codecov is not happy with the coverage (80%). It's just a few lines testing for proper headers in the files / right values in the right places. I don't really know how to test those lines.

The other two are unrelated to this PR:

================================== FAILURES ===================================
_________ test_make_inverse_operator_diag[testing_data-testing_data] __________
mne\minimum_norm\tests\test_inverse.py:749: in test_make_inverse_operator_diag
    _compare_inverses_approx(inverse_operator_diag, inv_op, evoked,
mne\minimum_norm\tests\test_inverse.py:183: in _compare_inverses_approx
    assert corr > ctol
E   assert 0.7766994175372354 > 0.99

[larsoner]

@fraimondo let me know when you're done and I'll see if I can simplify the _mult_cal_one stuff. I also see that info['chs'][ii]['unit'] is being used in a way that it shouldn't -- I'll add a check for that and fix it in this PR

[fraimondo]

I'm fine now. I think I got enough codecov. At least I don't see a failed test.

[agramfort]

cleanup?

[larsoner]

@fraimondo pushed a commit to simplify. Feel free to address @agramfort's remaining comment then hopefully we're good to go!

[agramfort]

Thx @fraimondo

[ftadel]

Hello,

A colleague posted a NK .EEG file I don't know how to read:

• https://neuroimage.usc.edu/forums/t/error-message-when-reviewing-raw-file-for-the-edf-files-generated-by-nihon-kohden/23798/11
• https://drive.google.com/file/d/1tGZGev8YZ6YooSZyLQfwKAbUSxV3ZL2u/view?usp=sharing

In case you have interesting sources of information about the NK file formats...

Thanks!
François

[agramfort]

MNE Python complains too:

In [3]: import mne
   ...:
   ...: raw = mne.io.read_raw_nihon('resting.eeg')
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-3-c17d6c9c0e8b> in <module>
      1 import mne
      2
----> 3 raw = mne.io.read_raw_nihon('resting.eeg')

~/work/src/mne-python/mne/io/nihon/nihon.py in read_raw_nihon(fname, preload, verbose)
     44     mne.io.Raw : Documentation of attribute and methods.
     45     """
---> 46     return RawNihon(fname, preload, verbose)
     47
     48

<decorator-gen-224> in __init__(self, fname, preload, verbose)

~/work/src/mne-python/mne/io/nihon/nihon.py in __init__(self, fname, preload, verbose)
    313         logger.info('Loading %s' % data_name)
    314
--> 315         header = _read_nihon_header(fname)
    316         metadata = _read_nihon_metadata(fname)
    317

~/work/src/mne-python/mne/io/nihon/nihon.py in _read_nihon_header(fname)
    134         version = np.fromfile(fid, '|S16', 1).astype('U16')[0]
    135         if version not in _valid_headers:
--> 136             raise ValueError(
    137                 'Not a valid Nihon Kohden EEG file ({})'.format(version))
    138

ValueError: Not a valid Nihon Kohden EEG file (BFA NKC04/01/16)