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

Add support for Biocam brw v4.x files, closes #1324 #1326

Merged
merged 3 commits into from
Nov 17, 2023

Conversation

mahlzahn
Copy link
Contributor

@mahlzahn mahlzahn commented Sep 6, 2023

  • missing: proper test file

@mahlzahn mahlzahn changed the title add support for Biocam brw v4.x files, closes #1324 Add support for Biocam brw v4.x files, closes #1324 Sep 7, 2023
@alejoe91
Copy link
Contributor

Thanks Robert! Can you generate and share a very small test file for the new format?

@mahlzahn
Copy link
Contributor Author

Thanks Robert! Can you generate and share a very small test file for the new format?

Hi Alessio, I will upload it and modify the concerning tests as soon as my colleague does new recordings. That should be until mid-October. (Same for SpikeInterface/probeinterface#216)

@matbonfanti
Copy link

Hi @mahlzahn, thanks for the work you have done! My collaborators and I are looking forward to using neo with our data.
Could we help you in any way? E.g. we could record and make available a short recording for the unittest...

@mahlzahn
Copy link
Contributor Author

@matbonfanti You can already try it if you checkout the branch (but this means you don't pick up the latest developments from master):

pip install git+https://github.com/mahlzahn/python-neo@add_brw_v4.x_support

@alejoe91 I created the test file and I am waiting for the next steps to do the upload, see https://gin.g-node.org/NeuralEnsemble/ephy_testing_data/issues/111.

Actually, I found there already another test file with a brw4.x file but not with the full raw data and this PR does not include support for such kind of recordings.

@matbonfanti
Copy link

thanks @mahlzahn, I have already tried your version of the code, reading a test recording from my collaborators. I can confirm it works fine (but this recording is too long to be used as test data).

@alejoe91
Copy link
Contributor

@matbonfanti You can already try it if you checkout the branch (but this means you don't pick up the latest developments from master):

pip install git+https://github.com/mahlzahn/python-neo@add_brw_v4.x_support

@alejoe91 I created the test file and I am waiting for the next steps to do the upload, see https://gin.g-node.org/NeuralEnsemble/ephy_testing_data/issues/111.

Actually, I found there already another test file with a brw4.x file but not with the full raw data and this PR does not include support for such kind of recordings.

That's great! If you send me the test files I can make a PR to gin. Let me know if instead you prefer to do it yourself!

@mahlzahn
Copy link
Contributor Author

That's great! If you send me the test files I can make a PR to gin. Let me know if instead you prefer to do it yourself!

I already prepared all and now I wait for the access rights and then I create the PR myself.

@alejoe91
Copy link
Contributor

@mahlzahn you should be able to open a PR now on GIN ;)

@pep8speaks
Copy link

pep8speaks commented Nov 2, 2023

Hello @mahlzahn! 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-02 09:59:25 UTC

@mahlzahn mahlzahn marked this pull request as ready for review November 2, 2023 09:59
@b-grimaud
Copy link
Contributor

Hi, any ETA on this ? I was trying to make my own version locally before finding this, great to see it being supported !

@alejoe91
Copy link
Contributor

This should be ready for final review since @mahlzahn affed the test files too!

@b-grimaud
Copy link
Contributor

Just noticed that, I also had a sample uploaded to the GIN a few months back.

Any reason not to calculate the gain with the bit depth as it is for brw v3.x ? I just get it with bit_depth = int(np.log2(rec_vars['ValueConverter']['MaxDigitalValue'])). They're very close numerically but on that kind of data I'm always a bit cautious.

I'll test with HerdingSpikes to see if there's the same scaling issues as with the original unfiltered neo readout.

@mahlzahn
Copy link
Contributor Author

Any reason not to calculate the gain with the bit depth as it is for brw v3.x ? I just get it with bit_depth = int(np.log2(rec_vars['ValueConverter']['MaxDigitalValue'])). They're very close numerically but on that kind of data I'm always a bit cautious.

Yes, the brw v4.x file format documentation explicitly states how to calculate the analog value on page 7:

AnalogValue = MinAnalogValue + DigitalValue * (MaxAnalogValue – MinAnalogValue) /
(MaxDigitalValue – MinDigitalValue)

I just followed this.

@b-grimaud
Copy link
Contributor

Opening, filtering and reading .brw v4.x worked just fine for me !

Still getting scaling issues as before :

  • Through neo/SI, no filtering :
    Spike_1855

  • Through neo/SI, with bandpass filter :
    Spike_858
    Spike_877

Some OK, some not, might just be saturation, hard to tell without the ability to filter in HerdingSpikes.

  • Compared to HerdingSpikes with my own sort of functional reader for brw v4.x :
    Spike_14080

Will try again with another sorter to see if this is HS-specific.

@b-grimaud
Copy link
Contributor

b-grimaud commented Nov 14, 2023

The issue seems to persist, even before sorting.

I loaded one of my recording, and through SpikeInterface :

traces = recording.get_traces(channel_ids=['45','100','1000','2486','78'], end_frame=4096)
plt.plot(traces)

Without filtering :

raw_traces

WIth bandpass filtering :
filtered_traces

Looks like the same 2048 scaling issue.
Would be curious to see if this is reproducible on someone else's recording to see if this could be due to some hardware/recording parameters issue.

EDIT :

Tested on the test file uploaded by @mahlzahn to GIN, similar results :

Raw traces :

gin_raw

Bandpass filtered :

gin_filtered

@samuelgarcia
Copy link
Contributor

Hi Robert.
thank you for adding this new format version.
Sorry for the delay of merging!

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.

6 participants