LorenFrankLab / spyglass

Neuroscience data analysis framework for reproducible research built by Loren Frank Lab at UCSF
https://lorenfranklab.github.io/spyglass/
MIT License
92 stars 42 forks source link

Unable to filter in LFP Band with referencing #98

Closed rsnevers closed 2 years ago

rsnevers commented 2 years ago

When referencing LFP data before filtering, there appears to be an error with indexing into the LFP data (line 495 - 496 in common_ephys). The data to be referenced is of size N, but when the reference signal is subtracted, the resulting array is size N x N. I don't know the exact details, but this appears to be caused by indexing into lfp_data with a whole list rather than just an index. I suspect someone just forgot to index into lfp_band_ref_index on line 496.

khl02007 commented 2 years ago

@rsnevers Could this be because one array is of size (1,N) and the other array is of size (N,1)? If you subtract one from the other in that case you get a (N,N) array.

edeno commented 2 years ago

+1 for @khl02007 's answer. You can use np.squeeze() to make sure the singleton dimension is gone.

rsnevers commented 2 years ago

@khl02007 This is probably what's happening, and I was able to fix things locally but am not exactly sure if my changes will generalize. There are a couple of lists of reference electrode indices, and I'm not sure which is supposed to be used. One of these lists is the output of the get_electrode_indices helper function, and it was actually never used anywhere in the method, so I assumed that this is the correct list of indices.

I can try submitting a pull request, but probably won't get around to it anytime soon since I'm going to step away from analysis stuff for about a week starting today.

lfrank commented 2 years ago

I think I fixed this in the lfp_ref_fix branch. Rhino - could you try it out and let me know?

On Dec 22, 2021, at 10:14 AM, rsnevers @.**@.>> wrote:

This Message Is From an External Sender This message came from outside your organization.

@khl02007https://urldefense.com/v3/__https://github.com/khl02007__;!!LQC6Cpwp!9r16iFH23WH54XcIV7T1Ypkc5pegzSQ19AG4I6OCMriFnF_oAWfvh9fiw8r0fjDNnA$ This is probably what's happening, and I was able to fix things locally but am not exactly sure if my changes will generalize. There are a couple of lists of reference electrode indices, and I'm not sure which is supposed to be used. One of these lists is the output of the get_electrode_indices helper function, and it was actually never used anywhere in the method, so I assumed that this is the correct list of indices.

I can try submitting a pull request, but probably won't get around to it anytime soon since I'm going to step away from analysis stuff for about a week starting today.

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/LorenFrankLab/nwb_datajoint/issues/98*issuecomment-999773159__;Iw!!LQC6Cpwp!9r16iFH23WH54XcIV7T1Ypkc5pegzSQ19AG4I6OCMriFnF_oAWfvh9fiw8rtciiJRQ$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ABV4PSOA2Y5YUOM5RVE566TUSIIRPANCNFSM5KRFSAIA__;!!LQC6Cpwp!9r16iFH23WH54XcIV7T1Ypkc5pegzSQ19AG4I6OCMriFnF_oAWfvh9fiw8qp74MRqg$. Triage notifications on the go with GitHub Mobile for iOShttps://urldefense.com/v3/__https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675__;!!LQC6Cpwp!9r16iFH23WH54XcIV7T1Ypkc5pegzSQ19AG4I6OCMriFnF_oAWfvh9fiw8rjDULt6Q$ or Androidhttps://urldefense.com/v3/__https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign*3Dnotification-email*26utm_medium*3Demail*26utm_source*3Dgithub__;JSUlJSU!!LQC6Cpwp!9r16iFH23WH54XcIV7T1Ypkc5pegzSQ19AG4I6OCMriFnF_oAWfvh9fiw8q0HX2aEA$. You are receiving this because you are subscribed to this thread.

lfrank commented 2 years ago

I believe this is all fixed.