Skip to content

Conversation

@tstenner
Copy link
Collaborator

@tstenner tstenner commented Mar 27, 2021

PR for #117.

  • Commit 1: add unit tests for postprocessing options
  • Commit 2: Move out smoothing parameters and smoothing code into a separate class that can be re-initialized and tested easier
  • Commit 3: reset postprocessing options when set_options() is called
  • Commit 4: adjust samples_seen when flushing samples

There's one major changes: the number of encountered samples is only tracked while proc_dejitter is active. Previously, samples were counted and enabling dejittering afterwards re-used this sample counter and therefore assumed a larger number of timestamps had been used to update the dejittering parameters.

@tstenner
Copy link
Collaborator Author

I think separating samples_seen and samples_since_t0 will make the estimation more robust with dropped samples, but that'll take some further tinkering.

@tstenner tstenner force-pushed the resetpostproc branch 3 times, most recently from e0a00da to c0a6b6e Compare April 20, 2021 18:04
@tstenner tstenner requested a review from cboulay April 20, 2021 18:12
@tstenner tstenner linked an issue Apr 22, 2021 that may be closed by this pull request
@cboulay
Copy link
Collaborator

cboulay commented Apr 26, 2021

Edit - replacing with better plots and updated gist...

@cboulay
Copy link
Collaborator

cboulay commented Apr 26, 2021

Updated: https://gist.github.com/cboulay/006bab4dc971cbd9c7b49e7d32c9cd30

This image was generated when using a conda environment containing a lsl.dll built on the previous version:
image

And this one when using the new lsl.dll
image
Don't worry about the constant offset of 0.6 msec - it's just because I'm using t[0] as my offset, even though t[0] itself is subject to postprocessing with no history so is a poor estimate.

So, in terms of fixing my problem, it looks good.

@cboulay
Copy link
Collaborator

cboulay commented Apr 27, 2021

One more time but only looking at the pre-flush stage to keep in scale for proper comparison of postprocessing functionality before and after the change.

Before change:
image

After change:
image

In addition to the above, I read through the code and unit tests, and I'm satisfied.

@cboulay cboulay merged commit 038fef0 into sccn:master Apr 27, 2021
@tstenner tstenner deleted the resetpostproc branch April 27, 2021 06:43
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.

Need ability to reset timestamp postprocessor state

2 participants