General API for handling sample gaps on rawio

[h-mayorquin]

@zm711, @samuelgarcia, and I had a discussion today about two current Blackrock issues (#1770 and #1755).

The main problem is that the gaps automatically detected by the code are much smaller than what would reasonably qualify as a segment in Neo (for example, someone deliberately pausing the recording). These gaps are more likely artifacts of the system (in #1770, it looks like a case of dropped samples).

Creating format-specific heuristics for each format would be a large maintenance burden. A simpler, more maintainable, and general solution is to provide users (who know their data best) with both information and control. We came up with the following proposal:

1. Default behavior: If gaps are detected, loading should error out. RawIO readers should raise an error when timestamp gaps are found and display a report showing the number, size, and characteristics of the gaps. This way, users can make an informed decision. Examples of this approach can be seen in Blackrock add summary of automatic data segmentation  #1769 and Improve intan reader error message for discontinuities #1484.
2. Opt-in behavior: RawIO readers should provide an argument (e.g. gap_tolerance_ms, naming TBD per reader) that lets users explicitly load data with gaps if they wish. Gaps smaller than the value will be ignored, gaps larger than the value will be segmented.
3. Timestamps from acquisition system: To make data with gaps more useful, we should provide the original timestamps from the acquisition system when available (see Add utility method to get timestamps on Intan base #1652 for Intan, BlackrockRawIO: Add _get_blackrock_timestamps for per-sample timestamp retrieval #1816 for Blackrock).
4. ignore_integrity_checks as the global escape hatch. A boolean kwarg at instantiation that bypasses every integrity check the reader performs, including gap detection and any format-specific file-validity checks. Default is False (strict). This serves two audiences: users who know their data is fine and want faster loads (integrity checks can be expensive, as @samuelgarcia pointed out), and users who need to open corrupt files to recover what they can (e.g. when the person who ran the experiment is no longer available). Examples: Add the possilibity of opening intan files even if corrupted. #1470 (Intan corrupt-file opening), Blackrock add block validation #1740 (other integrity checks).

This design would allow us to implement a consistent API across RawIO, so gaps are handled with a common interface. The plan is as follows:

1. Implement the solution first for Blackrock (since there are open issues and users currently cannot read the data). This will also let us to discuss the types and naming of the interface.
2. Extend the interface to other popular readers (e.g. Intan, OpenEphys, SpikeGLX, Plexon) to uncover potential difficulties.
3. Once the API is stable for the main readers, integrate it into the abstract/parent classes as a general RawIO API.
4. If possible, deprecate the current gap-handling mechanisms already in place (e.g. in OpenEphys) to simplify the codebase.

[EDIT, added mechanism to override the checks]

[samuelgarcia]

Thanks for the summary.
I would vote for gap_tolerance instead of segmentation_threshold.

[h-mayorquin]

Thanks for the summary. I would vote for gap_tolerance instead of segmentation_threshold.

I think that's a better name but I would like to add s or seconds at the end to make it more specific:

gap_tolerance_s

[zm711]

We voted at the maintenance meeting and we agreed to the general premise of this proposal. We want to:

1. Raises errors with gaps and provide informative errors about the nature of the gaps.
2. Ask the users to define their tolerance for gaps by setting a tolerance value which will then prevent an error from being raised and instead will segment at that tolerance level. gap_tolerance_s vs gap_tolerance_ms to be determined.
3. Add timestamps information for appropriate readers (barring any issues with keeping the rawio lazy)
4. Finally add a feature that skips these checks for users that want a fast reader capability and know their data is okay to not check for any issues (ignore_integrity_checks or potentially another name).

[h-mayorquin]

Great.

I think gap_tolerance_ms is better.

[PeterNSteinmetz]

I think whatever the unit is should be explicit, either in the name or as a parameter. I frankly prefer seconds. While many systems operate with ms as the underlying unit, others do not. For example, Neuralynx raw timestamps are microseconds.

The way I have dealt with this NeuralynxRawIO is by separating out the concept of an 'NcsSection' of contiguous samples with no gaps. There is a factory then to build these from a file. These are then used in the RawIO to present data in the standard way; however, a user can also use this to scan a file to detect when there are gaps. I would suggest a similar separation for the more general case. The options provided in the constructors then control how these sections are used to provide the more general interface.

[h-mayorquin]

I have been working on this and I think the default behavior is under-specified. We wrote above that RawIO readers should raise an error when timestamp gaps are found, but I think we need more precision: what about sub-resolution timestamp variation? For example, Blackrock PTP provides per-sample timestamps at nanosecond resolution, and natural clock jitter means consecutive sample intervals vary slightly around the expected period. These deviations are random, sub-sample, and clearly not missing data. One sample period is the natural boundary: below it, a deviation cannot represent a dropped sample.

I propose gap_tolerance_ms=None errors only on deviations >= 1 sample period (1.0 / sampling_rate). I think this default will reduce false positives by ignoring smaller-than-resolution timestamp variation while detecting meaningful dropped samples. The goal is to improve the user experience and reduce noise. Note that this leaves gap_tolerance_ms=0.0 as a strict mode that surfaces all detected deviations, including sub-sample ones, for debugging or timestamp quality inspection.

[samuelgarcia]

I am ok with!

[PeterNSteinmetz]

I think there are several issues to consider here.

Firstly, when there is a time gap between the blocks it does mean that something is not quite right. More specifically it means that if you take the timestamp of a block, which is in microseconds for Neuralynx, and add the number of samples in the block times the inverse of the stated sampling frequency, that there is more than a 1 microsecond error. This can happen due to improperly stated sampling frequencies, as in some of the files that were provided when the gap tolerance was introduced. (Do we yet have testing for that by the way?)

Secondly, my sense is that the RawIO is supposed to be a more technical interface. Given that it seems to me the RawIO level should by default be strictly correct.

Thirdly, since the normal number of samples in a .Ncs file block is 512, being 1/2 sample off in a block implies about a 0.2% error in timing. Over the course of a 30 minute experiment this can amount to a 3.6 second error. That can be quite significant if one has stimuli of a few seconds duration. This is something the user should be aware of and not simply glossed over.

Thus I wonder if rather than trying to make RawIO work more casually for the non-technical user it would be better to provide an interface at the Neo level which skips over things like gaps and so on by default. Then the average user can use those and not worry about it but those of us using the RawIO interface for more technical purposes will be warned of technical glitches, like gaps, unless we choose to override them.

Admittedly this is a matter of philosophy of approach.