bids-standard / pybv

A lightweight I/O utility for the BrainVision data format, written in Python.
https://pybv.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
21 stars 14 forks source link

Writing Events: Whitespace, Digit range, ... interaction with MNE-Python #24

Closed sappelhoff closed 5 years ago

sappelhoff commented 5 years ago

Our tests fail with MNE 0.18:

___________________________ test_bv_writer_oi_cycle ____________________________

    def test_bv_writer_oi_cycle():
        """Test that a write-read cycle produces identical data."""
        tmpdir = _mktmpdir()

        # Write, then read the data to BV format
        write_brainvision(data, sfreq, ch_names, fname, tmpdir, events=events,
                          resolution=np.power(10., -np.arange(10)))
        raw_written = mne.io.read_raw_brainvision(op.join(tmpdir, fname + '.vhdr'),
                                                  preload=True, stim_channel=False)
        # Delete the first annotation because it's just marking a new segment
        raw_written.annotations.delete(0)
        # Convert our annotations to events
        events_written, event_id = mne.events_from_annotations(raw_written)

        # sfreq
        assert sfreq == raw_written.info['sfreq']

        # Event timing should be within one index of originals
        assert_allclose(events[:, 0], events_written[:, 0], 1)
>       assert_array_equal(events[:, 1], events_written[:, 2])
E       AssertionError: 
E       Arrays are not equal
E       
E       Mismatch: 100%
E       Max absolute difference: 10000
E       Max relative difference: 0.99990001
E        x: array([1, 1, 2, 2])
E        y: array([10001, 10001, 10002, 10002])

pybv/tests/test_bv_writer.py:97: AssertionError

and in MNE 0.19.dev0, the stim_channel will be gone: https://github.com/mne-tools/mne-python/pull/6348/files

--> which makes this test fail as well:

=================================== FAILURES ===================================
___________________________ test_bv_writer_oi_cycle ____________________________

    def test_bv_writer_oi_cycle():
        """Test that a write-read cycle produces identical data."""
        tmpdir = _mktmpdir()

        # Write, then read the data to BV format
        write_brainvision(data, sfreq, ch_names, fname, tmpdir, events=events,
                          resolution=np.power(10., -np.arange(10)))
        raw_written = mne.io.read_raw_brainvision(op.join(tmpdir, fname + '.vhdr'),
>                                                 preload=True, stim_channel=False)
E       TypeError: read_raw_brainvision() got an unexpected keyword argument 'stim_channel'

pybv/tests/test_bv_writer.py:86: TypeError
sappelhoff commented 5 years ago

The solutions are simple

  1. simply remove the stim_channel=False argument
  2. we need to make sure that we always write events of length at LEAST 3 ... e.g., "S 2" instead of "S2"

point 2. is due to how MNE-Python handles BrainVision markers: https://github.com/mne-tools/mne-python/blob/a7b39388b3f5225df554cd7cf770a80b0136fe94/mne/io/brainvision/brainvision.py#L828-L829

--> however, this behavior is usually what BrainVision also implements (padding with whitespace to length 3: S 1, S 21, S123), although it seems to not be documented in the specification: https://www.brainproducts.com/productdetails.php?id=21&tab=5

... perhaps I'll write to customer support to include this. And also ask what happens when we have a four digit integer code S1234. Although that might never happen :thinking: rather, BrainVision would then have another prefix, e.g., R123

I'll submit a fix soon

sappelhoff commented 5 years ago

Update:

I just sent the following Email to the BP support team

Dear Brain Products support,

We are writing software than can write from Python to the BrainVision format, as specified on 
your website: https://www.brainproducts.com/productdetails.php?id=21&tab=5

The software is called pybv and is hosted on GitHub: https://github.com/bids-standard/pybv

We recently found an issue with the way how we write BrainVision events to the marker file. 
It seems that in all BrainVision files we have, the <description> of the marker is in the 
format: Letter, Digits padded with whitespace to be at least of length 3.

For example:

    S   1
    R 21
    S321

It is not specified in the BrainVision Core Data Format manual, that this whitespace padding 
is commonly used. So here are a few questions:

1. Is the whitespace padding mandatory? Or would a Marker like "S1" or "S34" also be okay?

2. What would happen for integers that are of length 4 or higher? For example "S1234" 
--> Would this still be valid? Or is the BrainVision format intended to host integer codes 
of a byte at maximum (i.e., 3 digits), and that some variability can be gained by changing
 the letter prefix - e.g. "A123", 
"B123", ...

We would be very happy to receive some answers, and if needed it would be great to 
get an update to the BrainVision Core Data Format manual.

Best,

Stefan on behalf of the pybv developer team
sappelhoff commented 5 years ago

It turns out that the situation is more complicated. For archiving reasons, I post the reply here:

Dear Stefan,

let me give you some background on this:
The special pattern that you correctly observed for the
markers created by BrainVision Recorder has to
do with the way that incoming triggers from the digital
port are interpreted in the context of the digital
trigger port settings. The number of available trigger port
bits depends on the amplifier (1, 8, 16 bits, 
f.ex.). For these bits, it is possible to define in
Recorder's trigger port settings different marker types 
(strings) that can be associated to each port bit. Those
bits of the port that are associated to the same 
marker type, are then grouped together to one binary number
that will be taken into the  the 
"description" of the resulting marker, preceded in addition
by the first letter of its type.

As an example: The default trigger port settings usually
associate the first half of the bits to "Stimulus" 
type triggers and the second half to "Response" type
triggers, which means that the binary values of 
the first set of bits are combined to the trigger number
for a stimulus marker (and will be preceded by 
an "S" in the marker description - the first letter of the
trigger type "Stimulus"), while the second set 
forms the value of the response marker ( preceded by an "R"
in the marker description), and both can 
be activated and stored simultaneously at the same time
point. This results, for example, in a marker of 
type "Stimulus" and name/description "S  1" - with some
whitespaces used for padding , as you 
correctly concluded.  So, the fact that some  markers are
of type "Stimulus" and that their names are 
equal to their number preceded by an "S" is due to the
settings chosen during recording.  Users have, 
however, the possibility to enter custom strings for the
type in Recorder's trigger port table - in this case 
the description of the generated markers will then start
with the first letter of the custom type.
The number space that is available depends on the number of
port bits that are assigned to one type 
during recording, which means in order to generate "S1234",
for example, you would need to associate 
at least 11 bits of the trigger port to a "type" that
starts with an "S" (f.ex. Stimulus, Sensory or 
Somethingelse ), and activate a pattern such as
10011010010. Applications that need a large amount 
of different trigger values could simply assign the full
set of 16 bits (f.ex.) to only one and the same 
type. The idea of splitting the digital port into sections,
though, is to allow different types of 
triggers/markers to be recorded in parallel - at the cost,
of course, of reducing the number of available 
bits for each type.

(See also the current Recorder manual chapter 6.2, page
62)

The above only describes the mechanism that is used during
Recorder's marker generation from the 
digital trigger port due to the natural technical
limitations. However, there is also an option to manually 
add comments and self-defined annotations to the recording,
where users can enter custom strings 
that are then turned into markers, which shows that other
types of strings are also supported.

The BrainVision Core Data Format obviously hosts all of the
above solutions and examples of marker 
creation, however, it is not restricted to these
limitations. It has a much wider and simpler definition
for 
marker type and description, reflected by the simple
statement "string" in the BrainVision Core Data 
Format document.

I hope it become clear why you mostly observe markers in
the typical format related to digital port 
mapping.

Please let me know if you have any further questions or
need more specific details.

With best regards
Lisa