Visitar URL original
General API for handling sample gaps on rawio · Issue #1773 · NeuralEnsemble/python-neo · GitHub
Skip to content

General API for handling sample gaps on rawio #1773

Open

Description

@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]

Activity

  1. changed the title [-]General API for handling data gaps on rawio[/-] [+]General API for handling sample gaps on rawio[/+] on Sep 10, 2025
  2. samuelgarcia commented on Sep 12, 2025

    @samuelgarcia
    Contributor

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

  3. h-mayorquin commented on Sep 12, 2025

    @h-mayorquin
    ContributorAuthor

    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

  4. zm711 commented on Sep 19, 2025

    @zm711
    Contributor

    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).
  5. h-mayorquin commented on Sep 19, 2025

    @h-mayorquin
    ContributorAuthor

    Great.

    I think gap_tolerance_ms is better.

  6. PeterNSteinmetz commented on Sep 19, 2025

    @PeterNSteinmetz
    Contributor

    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.

  7. added this to the 0.15.0 milestone on Dec 15, 2025
  8. h-mayorquin commented on Feb 18, 2026

    @h-mayorquin
    ContributorAuthor

    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.

  9. samuelgarcia commented on Feb 18, 2026

    @samuelgarcia
    Contributor

    I am ok with!

  10. PeterNSteinmetz commented on Feb 18, 2026

    @PeterNSteinmetz
    Contributor

    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.

  11. PeterNSteinmetz commented on Feb 18, 2026

    @PeterNSteinmetz
    Contributor

    There may also be an overload in this discussion on clock ticks and sampling frequency. Samples are often gathered on a coarser scale than the underlying clock mechanism permits. For example, in Neuralynx the timestamps are in microseconds but a typical sampling frequency of 24,000 has one sample every 41.677... microseconds.

  12. h-mayorquin commented on Apr 20, 2026

    @h-mayorquin
    ContributorAuthor

    Thanks Peter, and sorry for the delay. Other responsibilities took over but I want to push this forward.

    Two clarifying questions before proposing anything:

    1. When you say RawIO should be "strictly correct" by default, do you mean (a) every inter-record deviation should raise an error regardless of size, or (b) every deviation should be visible to the user but not necessarily block loading? These have very different implications.

    2. Your 3.6s-over-30-min concern is a drift argument: small per-record errors accumulating linearly. Is the worry that gap detection should catch that, or that some mechanism should and you're not convinced sampFreqUsed does the job? If the latter, I think we may agree on the problem and disagree only on the tool.


    Here's the framework I've been working from in PR #1822. I'd find it helpful to know whether you disagree with the framework itself or with the parameter choices inside it.

    I see three distinct mechanisms producing timestamp deviations in NCS files, each needing a different tool:

    Problem Mechanism Detectable by gap detection? Right tool
    Missing data Large timestamp jump (ms to hours) Yes gap_tolerance_ms (new): error by default, user opts into segmentation
    Hardware clock jitter Random per-record deviation (DLSX: std ~26 us, tails to +/-16 samples) Only with a threshold so tight it false-positives on jitter None, this is noise, not missing data
    Clock drift and integer-us rounding Fractional-us per-record error accumulating linearly No (per-record error is a fraction of a us) sampFreqUsed (pre-existing, divides total samples by actual elapsed us of the longest section, so a 0.2% clock error is absorbed into the time base) plus _get_neuralynx_timestamps() (new) for direct inspection

    My claim: gap detection only addresses row 1. The 3.6s/30min accumulation is row 3, and lowering the gap threshold does not improve that correction, it just floods the report. A real DLSX file I tested has 23,295 sub-sample deviations, none of them missing data; a strict detector would bury any actual dropout in that list. Happy to share the stats.

    That's why gap_tolerance_ms=None errors only at >= 1 sample period: below that, no dropped sample is physically possible, so the deviation can only be jitter or rounding. 0.0 remains available for inspecting every deviation.


    Questions for you:

    Happy to iterate.

  13. PeterNSteinmetz commented on Apr 20, 2026

    @PeterNSteinmetz
    Contributor

    Herberto, I agree there are 3 possible causes of error but think there is a 4th which should be distinguished.

    #4 That is that the hardware clock is simply not truly accurate. The gaps between records can be 0 and this still amounts to seconds over the course of a 30 min experiment because because of imperfect crystal controlled oscillators. This will only be noticed by people comparing the recorded times to some other time source, like internet time on a different machine.

    In terms of the Neo-wrapper versus RawIO, I can see your reasoning there though might not have done it that way myself. This seems to reflect a near deprecation of the Neo level, but I guess that will stay as it is. So long as the arguments and defaults are reasonable and clearly explained it should work.

    I agree that good defaults let #2 pass and #3 is really indistinguishable from #4 unless comparing to an outside clock.

    My own view is that the defaults should report any gaps greater than one sample time. Then the user knows about it and can adjust the parameter for gap sizes allowed within one segment. I guess distinguishing between None and 0 is a way to handle the show me every problem case, though it is perhaps a bit unusual. These sort of gaps normally indicate some mis-configuration of the recording hardware, at least in the case of Neuralynx.

    I have seen a lot of complaints about the gap reporting from users of SpikeInterface. I would suggest that at that level perhaps a looser level of defaults, which then invoke RawIO as appropriate, might work well for those users.

  14. PeterNSteinmetz commented on Apr 21, 2026

    @PeterNSteinmetz
    Contributor

    In terms of _get_neuralynx_timestamps, I think I personally would prefer simply exposing the underlying code concepts a bit more in the documentation. We already have an NcsSection and NcsSectionFactory as well as NlxHeader and NeuralynxRawIO._get_file_map.

    People who really want to understand the record structure of the file can easily use those to understand what is happening with the records in the file. I suppose we could provide some brief code snippets in the documentation on how to do that.

    Otherwise, for my own use I actually have classes NlxRawHeader and NcsRawFile which expose the NcsSections and NlxRawHeader for a file. We could add those to the neo.rawio.neuralynrawio package and explain those.

  15. h-mayorquin commented on Apr 21, 2026

    @h-mayorquin
    ContributorAuthor

    Thanks for taking a look. And you are right about the fourth mechanism: a clock that is wrong but stable is a distinct case I had missed from the framework. Thanks for completing it.

    On this in particular:

    In terms of the Neo-wrapper versus RawIO, I can see your reasoning there though might not have done it that way myself. This seems to reflect a near deprecation of the Neo level, but I guess that will stay as it is. So long as the arguments and defaults are reasonable and clearly explained it should work.

    Could you say more about what you mean by "near deprecation of the Neo level"? I want to make sure I am reading you correctly before I respond.

    My main goal here is to land a consistent API across the RawIO readers so that downstream consumers (NeoIO, SpikeInterface, neuroconv) can build on a single contract. I propagated this for Blackrock already in SpikeInterface #4453 and will do the same for Neuralynx once this lands. I am glad we landed in the same place on the 1-sample-period threshold for the default.

    On per-format leniency: I am grateful for your shepherding of Neuralynx, and if you think a different default makes sense given the nature of the acquisition system, I am fine with that at the Neuralynx level. The contract I care about keeping uniform is the API shape, not the specific default value. Concretely, I would like every reader to accept the same gap_tolerance_ms parameter where explicit float values mean the same thing everywhere ("segment at gaps above X ms"), while None can resolve to whatever sensible default each format warrants. That is exactly why None is useful as the default: it lets each reader pick its own meaning without forcing us to overload the float type with strings.

    Regarding exposing the underlying machinery you have built that describes the binary structure of the files: I agree there should be a how-to in the documentation covering NcsSection, NcsSectionsFactory, NlxHeader, and _get_file_map, but I do not think it replaces _get_neuralynx_timestamps(). A few reasons:

    • It uses the same (block_index, seg_index, stream_index) navigation that the rest of RawIO already uses, so downstream callers (SpikeInterface, neuroconv) get timestamps without having to learn a parallel file-level API.
    • It is segment-aware. If gap_tolerance_ms has split a file into multiple segments, the call returns just the timestamps for the requested segment. Walking _get_file_map directly would require the user to reconstruct the record-to-segment mapping themselves.
    • It matches the pattern we already established on Blackrock (_get_blackrock_timestamps) and Intan (get_intan_timestamps). Consistency across readers has more value here than a format-specific workflow for the common "give me raw timestamps" question.

    So my preference is to keep _get_neuralynx_timestamps() as the convenience entry point for the common case and add a how-to in the documentation that covers the full file-level introspection path for users who want it. What do you think?

  16. PeterNSteinmetz commented on Apr 21, 2026

    @PeterNSteinmetz
    Contributor

    Herberto, in terms of "near deprecation" what I mean is that it seems like many consumer libraries such as SpikeInterface, neuroconv, etc are using the RawIO interfaces instead of the NeoIO interfaces. Are there any that use that higher level interface anymore?

    I agree that a consistent API interface is valuable and certainly calling that parameter "gap_tolerance_ms" is reasonable. Though I do wonder if it should be in seconds everywhere rather than having to name it with "_ms". seconds are what is used for the other time values in the interface.

    In terms of _get_neuralynx_timestamps - do you want to generalize this as well to a name that makes sense for other of RawIO types. Maybe _get_section_timestamps or whatever you are generally calling this lower level concept below the segment.

  17. h-mayorquin commented on Apr 21, 2026

    @h-mayorquin
    ContributorAuthor

    Thanks Peter, and appreciate the quick turnaround.

    On "near deprecation": I honestly don't know, but I don't think so. My own work is on the SpikeInterface and neuroconv side (neuroconv in turn uses SpikeInterface), so I am less familiar with the NeoIO user base. I am sure the other maintainers have a wider view than I do, and I will defer to them on that.

    On seconds vs milliseconds: fair point on API consistency. My argument for ms is scale. The useful range for this parameter in practice is roughly 0.1 to 100 milliseconds, in seconds the same range becomes 0.0001 to 0.1:

    Scenario In ms In seconds
    Sub-sample threshold at 30 kHz 0.033 0.000033
    Single dropped sample 0.1 to 1.0 0.0001 to 0.001
    Typical USB buffer drop 1 to 10 0.001 to 0.01
    Merge small gap into a segment 10 to 100 0.01 to 0.1
    Pause or resume 1000+ 1.0+

    A misplaced decimal point in seconds hurts 1000x worse than in ms, and the _ms suffix doubles as a scale hint. The idea here is prioritizing user ergonomics rather than consistency with other conventions on the library.

    About the timestamps: this is work-in-progress and I am making private methods at the moment. I want to add private format-prefixed handles to each reader first (_get_blackrock_timestamps, _get_neuralynx_timestamps, _get_openephysbinary_timestamps, etc.) and then, once we have four or five in place, design the public unified accessor based on what we have actually learned. Return types already differ across readers (ns per sample in Blackrock PTP, clock ticks in Blackrock standard, integer µs in Neuralynx, sample indices elsewhere), and I do not want to lock in a common public name before knowing how to reconcile that. So first, some native timestamps in their own internal format and then after some ground work has been established we can design a public unified interface.

  18. PeterNSteinmetz commented on Apr 23, 2026

    @PeterNSteinmetz
    Contributor

    Herberto, I guess I come from enough of a physics background that I tend to like base units and then deal with the powers of ten. However, I imagine most neuroscientists aren't like that so as long as it is clearly labelled with the '_ms' cue, it seems like that should work well.

    A question regarding these private methods (which do seem like a reasonable approach to start with): what exactly are they going to return? The timestamp at the start of underlying blocks? If so, how will that be related to the sample positions?

  19. h-mayorquin commented on Apr 24, 2026

    @h-mayorquin
    ContributorAuthor

    Short answer: the private methods (_get_{format}_timestamps) are faithful to the formats and the public method (most likely get_signal_timestamps()) will return sampled aligned timestamps.

    More in detail, I am trying to keep _get_{format}_timestamps() as an internal tool that respects the format's native representation. So far where I have looked:

    • Neuralynx (_get_neuralynx_timestamps): per-record integer microseconds, one entry per 512-sample NCS record. That is what the file actually stores. Sample positions within a record are at uniform nominal spacing; a caller can derive per-sample times as record_ts[R] + sample_in_record * 1e6 / sample_rate if they need to.
    • OpenEphys Binary (_get_openephysbinary_timestamps): per-sample integer sample indices from sample_numbers.npy (or timestamps.npy pre-v0.6). One value per sample, 1:1 mapping.
    • Intan (get_intan_timestamps): per-sample integer sample indices from the file's timestamp field. One value per sample.

    There are kinks to iron out. Blackrock is the main one: across its variant history the acquisition system has stored timestamps very differently (per-sample nanoseconds in PTP, per-block ticks in standard formats, nothing at all in FileSpec 2.1), so "native and uniform within the format" cannot both hold there. I am still thinking about how to handle that.

    But that is the virtue of using private methods at the moment: once we have four or five formats in place, I will start thinking about unification. Overall that is the principle I would like to follow: private methods close to the file's internal representation, public uniform API on top. What do you think?

  20. PeterNSteinmetz commented on Apr 24, 2026

    @PeterNSteinmetz
    Contributor

    Herberto, this all sounds good and I agree there will be some kinks to sort out for a public facing method.

    Blackrock does sound like it will be a mess.

    One item to note regarding the Neuralynx format, which is sometimes overlooked, is that an Ncs record can contain up to 512 samples, but the actual number of valid samples could be less, and is stated in the record itself. I have only seen this in the last record in extant files, but it is possible that some weird writer could produce a file with a strange number of samples per record and then calculations of gaps need to take that into account.

  21. h-mayorquin commented on Apr 24, 2026

    @h-mayorquin
    ContributorAuthor

    Thanks for the heads up regarding that detail. I will add it to my notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions