Skip to content

Conversation

@mscheltienne
Copy link
Contributor

@mscheltienne mscheltienne commented Jan 9, 2026

Following #150- there was one assumption in this PR and of course it turns out to be wrong.. 😞

#150 assumes that the sample_count in the footer is correct; and thus it detects the anomaly by:

  1. Always truncates samples to match the footer sample_count
  2. Truncates the last clock offset only if it's statistically anomalous (see _detect_corrupted_clock_offset).

We found a file where the sample_count was n_sampels - 1, thus the last sample got truncated.. and yet it's a perfectly valid sample. Then workflow https://github.com/xdf-modules/pyxdf/actions/runs/20859146756/job/59933968045 highlights a case where an anomalous clock offset measurement is present, but without extra samples. So, this PR modifies the logic to:

  1. Check if there is an extra sample compared to the footer
  2. Check if there are anomalous/corrupted clock offsets statistically
  3. If both conditions are true, and only if both are true, we truncate both

@mscheltienne mscheltienne marked this pull request as ready for review January 10, 2026 19:11
@mscheltienne
Copy link
Contributor Author

@cbrnr This is good for review- I checked some more XDF files today. The updated logic is more conservative. It still correctly handles the synchronization bug reported by @JesseLivezey, where the clock measurements are correctly discarded- but it doesn't remove by default any extra samples compared to the footer information, which is often correct but not always (I found only one example where the footer information was erroneous while the samples were all valid; while I found many examples where the footer information was correct and the extra sample was indeed garbage).

@cbrnr cbrnr merged commit b5009ab into xdf-modules:main Jan 12, 2026
6 checks passed
@cbrnr
Copy link
Contributor

cbrnr commented Jan 12, 2026

Thanks @mscheltienne!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants