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
119 stars 126 forks source link

import xml info to bz_LoadBinary when user specs are missing #360

Open samamckenzie opened 4 years ago

samamckenzie commented 4 years ago

the defaults make the function work but give you garbage if you are not recording a single channel at 20kHz (is this anybody's default?)

I propose loading in from the xml when user specs are omitted

brendonw1 commented 4 years ago

That sounds like a bad default I believe the official buzcode way of reading from xml is bz_getSessionInfo

On Tue, Oct 29, 2019 at 10:10 PM samamckenzie notifications@github.com wrote:

the defaults make the function work but give you garbage if you are not recording a single channel at 20kHz (is this anybody's default?)

I propose loading in from the xml when user specs are omitted

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/buzsakilab/buzcode/issues/360?email_source=notifications&email_token=AA26WTIMGU4RUPRHEA2CEPDQRDUKJA5CNFSM4JGSJYSKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HVIKOYA, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA26WTI6VHSB3EP7RN5SQC3QRDUKJANCNFSM4JGSJYSA .

samamckenzie commented 4 years ago

We can either have bz_loadbinary and sessionInfo always yoked together, which means adding fields to sessionInfo for each dat file (time, auxiliary, analogin, digitalin, intracellular, juxtacellular, extracellular, ??), or we can make bz_loadbinary a more flexible function and have it read from the xml, since these always exist if the file was used in neuroscope.

I vote the latter.

I also vote to get rid of these defaults and let the function throw an error if the sessioninfo,/XML/user spec is omitted

I

brendonw1 commented 4 years ago

Getting rid of defaults makes sense IMO - whatever metadata system we have should automatically include all of what you ask for and should be present and set in every recording before processing This way the metadata guides all data properly and automatically - which forces the user to properly implement metadata as well. Good practice. That’s what I ask in my lab. Again my metadata system (in buzcode) covers all this.

Barring that, I’m fine w taking away the defaults and throwing errors

On Wed, Oct 30, 2019 at 7:18 AM samamckenzie notifications@github.com wrote:

We can either have bz_loadbinary and sessionInfo always yoked together, which means adding fields to sessionInfo for each dat file (time, auxiliary, analogin, digitalin, intracellular, juxtacellular, extracellular, ??), or we can make bz_loadbinary a more flexible function and have it read from the xml, since these always exist if the file was used in neuroscope.

I vote the latter.

I also vote to get rid of these defaults and let the function throw an error if the sessioninfo,/XML/user spec is omitted

I

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/buzsakilab/buzcode/issues/360?email_source=notifications&email_token=AA26WTIAVYZTND4KLGIDVQDQRFUOZA5CNFSM4JGSJYSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECTZFOI#issuecomment-547852985, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA26WTPWENL4ADNERPYQDJ3QRFUOZANCNFSM4JGSJYSA .

samamckenzie commented 4 years ago

How about these options

A) You and Peter integrate how sessioninfo handles all the meta data for the dat files and if the user asks to import a dat file for which an XML exists but a field in the sessionInfo does not, then we can make a new field based off the name of the dat file and re-save the meta data to the sessionInfo file.

For this to work we need a naming convention for the dat files, I suggest basename_fieldName.dat (e.g. basename_analogin.dat)

B) Alternatively, we can uncouple the two functions (bz_loadbinary and sessionInfo) and just load from the xml. I still prefer this option

C) Or we can force the user to provide channel number and Fs, but this decreases batch functionality when that data is already saved in the XML.

D) build a more general purpose wrapper that loads from the XML

dlevenstein commented 4 years ago

I think pulling from metadata as default is a good idea.

Whatever the format of the metadata file, I think we should call it the same way from within all functions in the repo. That way any changes to the metadata system can be made to that function.

Currently, metadata is called via bz_getSessionInfo, which does read from the xml if there's no sessionInfo file. So from my point of view, any calls to metadata should be made via bz_getSessionInfo, and updates to the metadata system should be made via updates to bz_getSessionInfo (which should be improved so it fits everyone's needs).

So, I vote A

samamckenzie commented 4 years ago

it currently reads from one XML, but not all of the XMLs.

Most of us never make an XML for time.dat nor auxiliary.dat, but one day we may want to read those files and make those XMLs. The question is do we force the user to do that every time for every session (even when that data is almost never used), or do we wait until the user wants to read in that data, then look for the XML and update the metadata. In which case, at some later point, the attempt at using bz_loadbinary will try to pull from the sessionInfo and find there is not field, then in bz_getsessioninfo there will be a flag to grab the relevant XML, which will then generate a new field. if no xml exists, then bz_getsessioninfo throws an error. under this framework, every dat file that is read by bz_loadbinary needs to be married to a field in sessionInfo - is this what we want? this sounds a bit onerous to me.

brendonw1 commented 4 years ago

Sorry to harp, but I really have done lots of this in my own metadata stuff in buzcode. I find myself annoyed that it's been totally ignored despite people asking for it 2.5 years ago. :) ...That said I now realize it won't be picked up by the lab because Peter has a nice system and so I'll assume we can take it apart and use the parts:

What I did was to read the rhd.info file to get all that info. If you want I can send blocks of text that extract that info about numbers of channels.

The other thing I have in there is code for ordering and grouping channels using .xlsx files (in the probeGeometries folder). .. and that populates the sessionInfo channels.

I don't use .xml initially, I flip the process around ... I use the other stuff first and I MAKE THE SESSIONINFO AND XML FROM MATLAB after some basic info is entered (Ie I have xml and sessionInfo maker functions on offer)

B

On Wed, Oct 30, 2019 at 11:30 AM samamckenzie notifications@github.com wrote:

it currently reads from one XML, but not all of the XMLs.

Most of us never make an XML for time.dat nor auxiliary.dat, but one day we may want to read those files and make those XMLs. The question is do we force the user to do that every time for every session (even when that data is almost never used), or do we wait until the user wants to read in that data, then look for the XML and update the metadata. In which case, at some later point, the attempt at using bz_loadbinary will try to pull from the sessionInfo and find there is not field, then in bz_getsessioninfo there will be a flag to grab the relevant XML, which will then generate a new field. if no xml exists, then bz_getsessioninfo throws an error. under this framework, every dat file that is read by bz_loadbinary needs to be married to a field in sessionInfo - is this what we want? this sounds a bit onerous to me.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/buzsakilab/buzcode/issues/360?email_source=notifications&email_token=AA26WTK7WAICSBNATVZH6P3QRGSAHA5CNFSM4JGSJYSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECUUNLA#issuecomment-547964588, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA26WTP3Z3ODHK3NTL75B43QRGSAHANCNFSM4JGSJYSA .

samamckenzie commented 4 years ago

this is perfect, better than reading from the XML https://github.com/buzsakilab/buzcode/blob/master/preprocessing/bz_DatFileMetadata.m

it just isn't integrated into the bz_loadbinary file.

we can adopt this standard rather than put this in sessionInfo. Up to you. But then this function needs to be read at the outset of the preprocessing pipeline, likely by bz_getsessioninfo

brendonw1 commented 4 years ago

Cool, yeah thanks for looking into it

And when you're ready to make sure that you have things arranged in shank/anatomical order you can use this (uses the .xlsx's in the ProbeGeometries folder): https://github.com/buzsakilab/buzcode/blob/master/preprocessing/metadata/bz_MakeXMLFromProbeMaps.m

And also there's this for writing more genreal xmls: https://github.com/buzsakilab/buzcode/blob/master/preprocessing/metadata/bz_MakeXML.m

the sessionInfo writing is buried somewhere - I could dig it up if we wanted it.

On Wed, Oct 30, 2019 at 3:47 PM samamckenzie notifications@github.com wrote:

this is perfect, better than reading from the XML

https://github.com/buzsakilab/buzcode/blob/master/preprocessing/bz_DatFileMetadata.m

it just isn't integrated into the bz_loadbinary file.

we can adopt this standard rather than put this in sessionInfo. Up to you. But then this function needs to be read at the outset of the preprocessing pipeline, likely by bz_getsessioninfo

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/buzsakilab/buzcode/issues/360?email_source=notifications&email_token=AA26WTPV4B47UEMZHVVMJ73QRHQDXA5CNFSM4JGSJYSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECVREDQ#issuecomment-548082190, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA26WTM4KLBIUSZIK2JYUDLQRHQDXANCNFSM4JGSJYSA .

evilrobotxoxo commented 4 years ago

I haven’t been following this closely, but I was wondering if all the metadata stuff is currently intan-dependent. We are probably switching to open ephys for our acquisition, with both the Intan system and Neuropixels, and we could help make the systems compatible.

On Wed, Oct 30, 2019 at 3:52 PM Brendon Watson notifications@github.com wrote:

Cool, yeah thanks for looking into it

And when you're ready to make sure that you have things arranged in shank/anatomical order you can use this (uses the .xlsx's in the ProbeGeometries folder):

https://github.com/buzsakilab/buzcode/blob/master/preprocessing/metadata/bz_MakeXMLFromProbeMaps.m

And also there's this for writing more genreal xmls:

https://github.com/buzsakilab/buzcode/blob/master/preprocessing/metadata/bz_MakeXML.m

the sessionInfo writing is buried somewhere - I could dig it up if we wanted it.

On Wed, Oct 30, 2019 at 3:47 PM samamckenzie notifications@github.com wrote:

this is perfect, better than reading from the XML

https://github.com/buzsakilab/buzcode/blob/master/preprocessing/bz_DatFileMetadata.m

it just isn't integrated into the bz_loadbinary file.

we can adopt this standard rather than put this in sessionInfo. Up to you. But then this function needs to be read at the outset of the preprocessing pipeline, likely by bz_getsessioninfo

— You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/buzsakilab/buzcode/issues/360?email_source=notifications&email_token=AA26WTPV4B47UEMZHVVMJ73QRHQDXA5CNFSM4JGSJYSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECVREDQ#issuecomment-548082190 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AA26WTM4KLBIUSZIK2JYUDLQRHQDXANCNFSM4JGSJYSA

.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/buzsakilab/buzcode/issues/360?email_source=notifications&email_token=ABG4WJKFFS5LGIJ7YH2PDEDQRHQYHA5CNFSM4JGSJYSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECVRT7A#issuecomment-548084220, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABG4WJLG3IZD3UGHBJZHJVDQRHQYHANCNFSM4JGSJYSA .

-- Sent from my iPhone

brendonw1 commented 4 years ago

The way it's written now is tolerant to Amplipex or Intan. Someone just needs to add another case and it should work with OpenEphys

On Wed, Oct 30, 2019 at 6:20 PM Luke Sjulson notifications@github.com wrote:

I haven’t been following this closely, but I was wondering if all the metadata stuff is currently intan-dependent. We are probably switching to open ephys for our acquisition, with both the Intan system and Neuropixels, and we could help make the systems compatible.

On Wed, Oct 30, 2019 at 3:52 PM Brendon Watson notifications@github.com wrote:

Cool, yeah thanks for looking into it

And when you're ready to make sure that you have things arranged in shank/anatomical order you can use this (uses the .xlsx's in the ProbeGeometries folder):

https://github.com/buzsakilab/buzcode/blob/master/preprocessing/metadata/bz_MakeXMLFromProbeMaps.m

And also there's this for writing more genreal xmls:

https://github.com/buzsakilab/buzcode/blob/master/preprocessing/metadata/bz_MakeXML.m

the sessionInfo writing is buried somewhere - I could dig it up if we wanted it.

On Wed, Oct 30, 2019 at 3:47 PM samamckenzie notifications@github.com wrote:

this is perfect, better than reading from the XML

https://github.com/buzsakilab/buzcode/blob/master/preprocessing/bz_DatFileMetadata.m

it just isn't integrated into the bz_loadbinary file.

we can adopt this standard rather than put this in sessionInfo. Up to you. But then this function needs to be read at the outset of the preprocessing pipeline, likely by bz_getsessioninfo

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <

https://github.com/buzsakilab/buzcode/issues/360?email_source=notifications&email_token=AA26WTPV4B47UEMZHVVMJ73QRHQDXA5CNFSM4JGSJYSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECVREDQ#issuecomment-548082190

, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AA26WTM4KLBIUSZIK2JYUDLQRHQDXANCNFSM4JGSJYSA

.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub < https://github.com/buzsakilab/buzcode/issues/360?email_source=notifications&email_token=ABG4WJKFFS5LGIJ7YH2PDEDQRHQYHA5CNFSM4JGSJYSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECVRT7A#issuecomment-548084220 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/ABG4WJLG3IZD3UGHBJZHJVDQRHQYHANCNFSM4JGSJYSA

.

-- Sent from my iPhone

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/buzsakilab/buzcode/issues/360?email_source=notifications&email_token=AA26WTPR4UGK45HV3HMDL53QRICBRA5CNFSM4JGSJYSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECV6RMY#issuecomment-548137139, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA26WTPFOR6RT76BPZ6E3EDQRICBRANCNFSM4JGSJYSA .