Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 2048 µv offset and signal inversion for BioCAM recordings #1348

Closed
wants to merge 0 commits into from

Conversation

b-grimaud
Copy link
Contributor

Closes #1347.
Will update with BRW v4.x support once #1326 is approved.

@pep8speaks
Copy link

pep8speaks commented Nov 15, 2023

Hello @BptGrm! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2023-11-15 18:09:30 UTC

@alejoe91
Copy link
Contributor

@BptGrm before you submit the PR. I discussed with @samuelgarcia about this.

We think that the biocam system has a 12-bit ADC, so that's where the 4096 come from. In general, NEO tends to keep the original data type (at least the RawIO), and use the gain and offset property to tell how to convert to uV. In this case, the offset should be half of the dynamic range: 2^11 = 2048.

Does it make sense?
We should deal with the "conversion" later in the pipeline. SpikeInterface has a nice unsigned_to_signed function (see here), that we could extend with an optional bit_depth (if None, it will take the bit depth from the data type, e.g., int16 -> 16). Does it make sense?

@alejoe91
Copy link
Contributor

In other words, the 2048 offset is correct. That is exactly how the data is saved to disk ;)

@b-grimaud
Copy link
Contributor Author

Makes sense !

I guess the conversion should happen later on, probably in SpikeInterface's BioCAM extractor ? I'm not that familiar with those.

It would probably be safer to have it on by default with the option to turn it off.

Should this still be mentionned in the comments or the docs of the neo reader, in case it is used outside of SI ?

@b-grimaud
Copy link
Contributor Author

I can also open a smaller PR just for handling the signal inversion mistake, that should still be fixed although I don't think it's a problem with current 3Brain recordings.

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.

Filter-dependant voltage shift in BioCAM recordings
3 participants