buzsakilab / buzcode

Code for internal lab sharing - polishing has started but is by no means complete
http://www.buzsakilab.com/
GNU General Public License v3.0
118 stars 128 forks source link

Buzsaki functions for NWB file format #319

Open mpompolas opened 5 years ago

mpompolas commented 5 years ago

Buzsaki functions converted to work with the NWB file format.

The functions have the same input and produce the same structured outputs. It is assumed that all the information of a recording has already been converted correctly to a NWB file and the functions get info from the appropriate fields.

Example calls of the functions are at the beginning of each file.

testing_BUZSAKI_NWB_functions_fidelity.m has calls for both existing Buzcode functions and the NWB counterparts that I used for easy comparison.

The functions were tested on this file: https://drive.google.com/file/d/1ceoaieEpn7NVmU36o-yRTWwJFYjjWdJQ/view

Problem with this dataset: The .map field in: behavior.events.map takes a long time to compute in this dataset since the trials are long (probably too long) and the default bin_size for the .map is set to 200 - Consider increasing the bin size

bendichter commented 5 years ago

Hey @DavidTingley, @dlevenstein, @brendonw1, etc.

@mpompolas has been contributing to our effort to bring NWB into buzcode. For many labs, we are working with them to build converters into NWB. For the Buzsaki Lab, we are trying to go further, working with your analysis to incorporate it directly with NWB. This will 1) Allow you to use buzcode on NWB files released by your lab or other labs 2) Allow other labs to use buzcode on their own NWB data 3) Ensure that the data in NWB files are sufficient for these analyses

We are developing/validating NWB for your pipeline in a way so that you can always revert to using the buzcode format, so we are not risking breaking your analysis pipeline. We think at this point we have built the necessary IO functions (analogs to already existing IO functions) so that the buzcode suite can be used on NWB files. At this point, it would be very helpful if you could take a look at these changes and let us know if anything is missing or if you need any more information. This is the first of hopefully many NWB lab integrations, and we are using this experience to learn the best way to do it, so feedback is definitely welcome!

dlevenstein commented 5 years ago

Hi all - would it be better to build this into the the existing I/O files? That way as we continue to develop we don't need to update two sets of load functions.

brendonw1 commented 5 years ago

I in theory like it being integrated - but it may be a lot of work... I guess if we start one at a time it's not so bad

On Mon, Apr 29, 2019 at 2:22 PM Dan Levenstein notifications@github.com wrote:

Hi all - would it be better to build this into the the existing I/O files? That way as we continue to develop we don't need to update two sets of load functions.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/buzsakilab/buzcode/pull/319#issuecomment-487689597, or mute the thread https://github.com/notifications/unsubscribe-auth/AA26WTJDTIPV2G6IPPH22HLPS44IDANCNFSM4HISDDMQ .

dlevenstein commented 5 years ago

I expect that as we continue to develop, we'll update the buzcode I/O functions and no one will keep the NWB functions up to date.

We are already running into this issue with the phy spikes import function, which needs to be incorporated into bz_getSpikes (issues #264 and #261).

Happy to merge the pull request, but I have a hard time seeing this as the right approach moving forward... none of us have/are using NWB data for daily analysis and that's the only way any of this stays up to date. IMO A better approach would be to have a file-type switch in a single load function, which pipes to unified formatting step as early as possible before output.

bendichter commented 5 years ago

@dlevenstein and @brendonw1 thanks for taking a look at this and thinking critically about how it will integrate with your workflow. @dlevenstein, your request to integrate this into current IO functions makes sense. I can see how having multiple IO functions might be difficult to maintain. @mpompolas and I will discuss today and get back you you about this.

mpompolas commented 5 years ago

@dlevenstein @brendonw1
At this point I'm familiar with both importers so it shouldn't be too hard for me to add a file-type switch and integrate everything together.

Let me get back to you after I chat with @bendichter .

mpompolas commented 5 years ago

Ok, I'll take care of the merge of both importers and get back to you by next week.

My plan is to add a file-type switch, to check if there is a .nwb file present on the basefolder/current folder. If there is, it will give priority to the .nwb info (I assume here that users would convert all the relevant files to a single .nwb file after they are finalized - unless you guys object and prefer the other files to be prioritized).

dlevenstein commented 5 years ago

Please don't default to nwb.

mpompolas commented 5 years ago

Ok, which branch should I use? dev I assume?

dlevenstein commented 5 years ago

yup - dev is good. Thanks!

brendonw1 commented 5 years ago

Thank you very much.

On Wed, May 1, 2019 at 5:12 PM Dan Levenstein notifications@github.com wrote:

yup - dev is good. Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/buzsakilab/buzcode/pull/319#issuecomment-488430016, or mute the thread https://github.com/notifications/unsubscribe-auth/AA26WTPK5VVZZBKZ4IHK4ZLPTIBUFANCNFSM4HISDDMQ .

mpompolas commented 5 years ago

Hi all, I updated the Buzcode functionality with NWB support according to your last suggestion for using a single function for both current files and NWB files. The functions (bz_GetLFP, bz_GetSpikes etc.) load the files as usual, and if the standard files can not be found, it checks for the equivalent fields in an NWB file. Users need to fill the appropriate fields of the NWB file for the functions to work. I chatted with Peter and Roman last week at Janelia, and they gave me a better understanding of your lab needs for setting up the NWB files. I reorganized the way that users can fill the NWB files based on their suggestions, with functions that store the files that follow the Buzcode standard format, and functions that will be specific for each user (this will be located in a separate subfolder for each lab member). The functions that are meant for the files that follow the standard Buzcode format are tested on the tutorial dataset: https://github.com/buzsakilab/buzcode/tree/master/tutorials/exampleDataStructs/20170505_396um_0um_merge In order to create your own NWB file of the tutorial dataset, just call the script: example_script_to_add_fields_from_Buzcode_format

I also added a folder for example functions to store the YutaMouse41-150903 dataset into an NWB file. The functions that are only for this user, are indicated with the suffix Yuta (e.g. addEvents_Yuta). Note that this dataset doesn’t have a *spikes.cellinfo.mat file, but has the .res, .spk etc. files. I added a function in the Neuroscope folder that stores spiking information (and waveforms – there is a flag you can deactivate if you don’t want them) straight from the Neuroscope files. To create the NWB file of the YutaMouse41 dataset, just run: /Yuta/YutaMouse41_toNWB

Please take a look at the updated functions and let me know if you have any comments.

mpompolas commented 5 years ago

I would need some guidance on the detectorinfo.detectionparms field within *events.mat file. Can someone tell me what fields are mandatory there? (this structure seems to store a lot of information and is not clear to me how I should proceed to generalize it).

Thanks

brendonw1 commented 5 years ago

This seems great. Can you guys who have been discussing all of this give some explanation of the imagined workflow? Or how NWB would be used in this context?

On Wed, May 22, 2019 at 1:06 AM Konstantinos notifications@github.com wrote:

I would need some guidance on the detectorinfo.detectionparms field within *events.mat file. Can someone tell me what fields are mandatory there? (this structure seems to store a lot of information and is not clear to me how I should proceed to generalize it).

Thanks

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/buzsakilab/buzcode/pull/319?email_source=notifications&email_token=AA26WTJWFR3FSO5RJAWTCBLPWTIEPA5CNFSM4HISDDM2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODV55B5A#issuecomment-494653684, or mute the thread https://github.com/notifications/unsubscribe-auth/AA26WTITNMIM3OWYZOWLZE3PWTIEPANCNFSM4HISDDMQ .

mpompolas commented 5 years ago

The NWB format enables the entire lab to use a universal language. You guys have already done a great job at standardizing your formats but as far as I understand there are still differentiations between users. The idea would be for every person to be able to use all other members' data, even if they were saving files in a different format. Of course, attention needs to be put on how people put their files in NWB: Probably there needs to be a discussion on what is essential to be saved on an NWB and all datasets are guaranteed to have these specified fields.

That said, the scope of NWB goes further than the Buzsaki lab. You will be able to use data from other labs since, as I mentioned before, it would be in the same universal language. I personally have supported NWB integration within Brainstorm as part of the invasive neurophysiology toolbox: https://neuroimage.usc.edu/brainstorm/e-phys/Introduction

Other tools have already been adopting NWB integration, as presented last week at Janelia, so you can potentially use these as well: https://neurodatawithoutborders.github.io/tools

brendonw1 commented 5 years ago

Thanks so much Konstantinos. That sounds about like my understanding actually of NWB and I love the idea.

I’m wondering though if this community has thoughts on interfacing buzcode formats w NWB - in terms of pragmatic workflow. I guess I’m assuming we stick w buzcode for initial analysis and then switch to NWB for sharing data later.

On Thu, May 23, 2019 at 8:16 PM Konstantinos notifications@github.com wrote:

The NWB format enables the entire lab to use a universal language. You guys have already done a great job at standardizing your formats but as far as I understand there are still differentiations between users. The idea would be for every person to be able to use all other members' data, even if they were saving files in a different format. Of course, attention needs to be put on how people put their files in NWB: Probably there needs to be a discussion on what is essential to be saved on an NWB and all datasets are guaranteed to have these specified fields.

That said, the scope of NWB goes further than the Buzsaki lab. You will be able to use data from other labs since, as I mentioned before, it would be in the same universal language. I personally have supported NWB integration within Brainstorm as part of the invasive neurophysiology toolbox: https://neuroimage.usc.edu/brainstorm/e-phys/Introduction

Other tools have already been adopting NWB integration, as presented last week at Janelia, so you can potentially use these as well: https://neurodatawithoutborders.github.io/tools

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/buzsakilab/buzcode/pull/319?email_source=notifications&email_token=AA26WTPQXPYCH7JHLKZR5J3PW4XVVA5CNFSM4HISDDM2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWD2GVY#issuecomment-495428439, or mute the thread https://github.com/notifications/unsubscribe-auth/AA26WTNINABJCT75WTQPXJTPW4XVVANCNFSM4HISDDMQ .

mpompolas commented 5 years ago

I can't speak for the NWB community in general, but the way that I have it in my mind, the main purpose of the NWB is to make sure that everybody understands what happened during the acquisition. Every aspect of the experiment is logged in detail within the NWB. Of course you can add processed data on it as well, but the way the processing is done can vary vastly among labs. The standardization of the analysis though is exactly the effort we are making with Brainstorm. Potentially adding Buzcode analysis functionality within the Brainstorm workflow is of great interest to me, so maybe we can have a chat about it. Let me know what you think.