diff --git a/CHANGELOG.md b/CHANGELOG.md index fcfb5bb5..d9cee8db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -197,6 +197,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Fixed +- **Reading samples through a DAQ system: raw and ingested epochs now agree.** + `ndi_daq_system_mfdaq.readchannels_epochsamples` takes 0-based samples, as + `epochtimes2samples` and `epochsamples2times` do, but passed them unchanged + to a raw reader, whose samples are 1-based. A raw epoch therefore read one + sample later than the same epoch ingested, and a probe's raw data + (`ndi_probe_timeseries_mfdaq.readtimeseriesepoch`) were one sample off from + the times returned with them. The system now converts for the raw reader. + - **`session.daqsystem_add` raised AttributeError for every DAQ system**: it called `dev.setsession`, but `ndi_daq_system`'s method is `set_session`. diff --git a/src/ndi/daq/ndi_matlab_python_bridge.yaml b/src/ndi/daq/ndi_matlab_python_bridge.yaml index fed6f8e4..97774777 100644 --- a/src/ndi/daq/ndi_matlab_python_bridge.yaml +++ b/src/ndi/daq/ndi_matlab_python_bridge.yaml @@ -1282,7 +1282,16 @@ classes: - name: data type_python: "np.ndarray" decision_log: > - Semantic Parity: channel, epoch_number, s0, s1 are 1-indexed. + Semantic Parity: channel and epoch_number are 1-indexed. s0 and s1 + are sample positions, 0-based as in the rest of this port's sample + API (epochtimes2samples, epochsamples2times; see + docs/developer_notes/ndi_xlang_principles.md and NDI-python#313), + where MATLAB's are 1-based. A raw reader's readchannels_epochsamples + takes 1-based samples, so they are converted before it is called; the + ingested reader takes 0-based samples. Until this was fixed the raw + path passed the 0-based numbers through unchanged, so a raw epoch read + one sample later than the same epoch ingested, and a probe's raw data + were one sample off from the times it returned with them. - name: readevents_epochsamples input_arguments: diff --git a/src/ndi/daq/system_mfdaq.py b/src/ndi/daq/system_mfdaq.py index 0ce843e7..946830e5 100644 --- a/src/ndi/daq/system_mfdaq.py +++ b/src/ndi/daq/system_mfdaq.py @@ -136,12 +136,20 @@ def readchannels_epochsamples( """ Read channel data by sample indices. + Note: + Unlike MATLAB (1-based), Python sample indices are 0-based: + sample 0 is the epoch's first sample, as for + :meth:`epochtimes2samples` and :meth:`epochsamples2times`. A raw + reader's ``readchannels_epochsamples`` takes 1-based samples, so + they are converted here; an ingested epoch's reader takes 0-based + samples already. Both paths read the same samples. + Args: channeltype: Channel type(s) (e.g., 'ai', 'analog_in') channel: Channel number(s) (1-indexed) epoch_number: ndi_epoch_epoch number (1-indexed) - s0: Start sample (1-indexed) - s1: End sample (1-indexed) + s0: Start sample (0-based) + s1: End sample (0-based) Returns: Array with shape (num_samples, num_channels) @@ -157,7 +165,10 @@ def readchannels_epochsamples( return self._daqreader.readchannels_epochsamples_ingested( channeltype, channel, epochfiles, s0, s1, self.session ) - return self._daqreader.readchannels_epochsamples(channeltype, channel, epochfiles, s0, s1) + # the raw reader's samples are 1-based + return self._daqreader.readchannels_epochsamples( + channeltype, channel, epochfiles, s0 + 1, s1 + 1 + ) def readevents_epochsamples( self, diff --git a/src/ndi/probe/timeseries_mfdaq.py b/src/ndi/probe/timeseries_mfdaq.py index 358deeb6..ad1be6be 100644 --- a/src/ndi/probe/timeseries_mfdaq.py +++ b/src/ndi/probe/timeseries_mfdaq.py @@ -50,8 +50,8 @@ def read_epochsamples( Args: epoch: ndi_epoch_epoch number (1-indexed) or epoch_id - s0: Start sample (1-indexed) - s1: End sample (1-indexed) + s0: Start sample (0-based, as epochtimes2samples returns) + s1: End sample (0-based) Returns: Tuple of (data, t, timeref_out): diff --git a/tests/test_daq_system_sample_indexing.py b/tests/test_daq_system_sample_indexing.py new file mode 100644 index 00000000..618da597 --- /dev/null +++ b/tests/test_daq_system_sample_indexing.py @@ -0,0 +1,86 @@ +"""ndi_daq_system_mfdaq.readchannels_epochsamples reads the same samples +whether the epoch is raw or ingested. + +Sample positions are 0-based in this port (docs/developer_notes/ +ndi_xlang_principles.md): sample 0 is the epoch's first sample, as for +epochtimes2samples and epochsamples2times. A raw reader's +readchannels_epochsamples takes 1-based samples and the ingested reader +0-based ones; the DAQ system used to pass its numbers to both unchanged, so a +raw epoch read one sample later than the same epoch ingested. +""" + +from __future__ import annotations + +import shutil + +import numpy as np +import pytest + +from ndi.gui.component.ProgressBarWindow import ndi_gui_component_ProgressBarWindow as PBW +from tests.test_ingest_progress_parallel import _have_intan, _intan_session, _prefs + +pytestmark = pytest.mark.skipif(not _have_intan(), reason="Intan example data (.rhd) not found") + + +@pytest.fixture(autouse=True) +def _silent(): + previous = PBW.silentModeDefault() + PBW.silentModeDefault(True) + yield + PBW.silentModeDefault(previous) + + +def _daq(session): + d = session.daqsystem_load(name="intan1") + return d[0] if isinstance(d, list) else d + + +def _epoch_number(daq, epoch_id): + return next(e["epoch_number"] for e in daq.epochtable() if e["epoch_id"] == epoch_id) + + +def test_raw_read_starts_at_sample_zero(tmp_path, monkeypatch): + _prefs(monkeypatch, OpenPoolIfNone=False) + daq = _daq(_intan_session(tmp_path / "exp1")) + files = daq.epochtable()[0]["underlying_epochs"]["underlying"] + first_five = daq.daqreader.readchannels_epochsamples("ai", [1, 2], files, 1, 5) + np.testing.assert_array_equal(daq.readchannels_epochsamples("ai", [1, 2], 1, 0, 4), first_five) + + +def test_raw_time_channel_matches_epochsamples2times(tmp_path, monkeypatch): + # what a probe returns: data read at samples s0..s1, times from + # epochsamples2times(s0..s1); the two must describe the same samples + _prefs(monkeypatch, OpenPoolIfNone=False) + daq = _daq(_intan_session(tmp_path / "exp1")) + samples = np.arange(10, 20) + t_read = np.ravel(daq.readchannels_epochsamples("time", [1], 1, 10, 19)) + t_calc = np.ravel(daq.epochsamples2times("time", [1], 1, samples)) + np.testing.assert_allclose(t_read, t_calc, rtol=0, atol=1e-9) + + +def test_raw_and_ingested_read_the_same_samples(tmp_path, monkeypatch): + from ndi.session.dir import ndi_session_dir + + _prefs(monkeypatch, OpenPoolIfNone=False) + raw = _intan_session(tmp_path / "raw") + _daq(raw).epochtable() # write the epoch ids, so the copy shares them + shutil.copytree(tmp_path / "raw", tmp_path / "ingested") + raw = ndi_session_dir("exp1", tmp_path / "raw") + ing = ndi_session_dir("exp1", tmp_path / "ingested") + ok, msg = ing.ingest() + assert ok, msg + ing.cache.clear() + + d_raw, d_ing = _daq(raw), _daq(ing) + for entry in d_raw.epochtable(): + n_raw = entry["epoch_number"] + n_ing = _epoch_number(d_ing, entry["epoch_id"]) + for channeltype, channels in (("ai", [1, 2]), ("time", [1])): + for s0, s1 in ((0, 4), (100, 140)): + np.testing.assert_allclose( + d_raw.readchannels_epochsamples(channeltype, channels, n_raw, s0, s1), + d_ing.readchannels_epochsamples(channeltype, channels, n_ing, s0, s1), + rtol=0, + atol=1e-9, + err_msg=f"{channeltype} samples {s0}..{s1}", + )