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

Consolidate bz_Comodulogram.m, PhaseAmpCouplingByAmp.m, bz_ModIndex.m, bz_PhaseAmplitudeDist.m duplicates #292

Open DavidTingley opened 5 years ago

DavidTingley commented 5 years ago

We need to consolidate these functions, add some additional input options, and clean up the usage sections.

AntonioFR8 commented 5 years ago

I'm working on this now. Will make new functions for cross-frequency coupling

dlevenstein commented 4 years ago

All should output same thing. Compatible with bz_WaveSpec.

dlevenstein commented 4 years ago

Add Farnaz and Elie

dlevenstein commented 4 years ago

@eliezyer not sure how to add you here, or how to find Farnaz' account, so tagging you here

dlevenstein commented 4 years ago

I'd say the first thing to do is take stock of the phase/amp coupling functions - we should not delete those that use different algorithms at this point. But we should make sure it's clear what each does, and how it's used. We can then decide if we want to consolidate any

eliezyer commented 4 years ago

Sounds good, I will take a look on this tomorrow. A quick search and I saw that the method I used in bz_ModIndex.m and the one Antonio did are the same. So, i'll quickly test with a recording to see if I find any differences otherwise we can delete one of them.

I also think it would be better to rename (in case we don't consolidate) these functions to reflect their main analysis (amp-amp, phase-amp and etc)

eliezyer commented 4 years ago

Okay, so here is the thing:

1) bz_CrossFreqMod (Which is not listed in this issue) is a duplicate of bz_ModIndex (phase of multiple low-frequencies against amplitude of multiple high-frequencies). The difference between them is that the first one gives the opportunity to calculate the CFC across many channels, using a fixed channel for the phase and changing the channels for the amplitude. I will keep that functionality and make the function more general (allowing the user to have access to more inputs and etc).

2) bz_PhaseAmplitudeDist CFC (phase-amp) of a single low-freq(phase) to multiple high-freq(amp). Very similar method to the functions in 1), filtering & wavelet. If we change the way that the phase amplitude map is normalized (sum of the map instead of mean) I can call/use that function from inside the function done in 1). TBH, the CFC from 1) is a duplicate/enhancement of this function but normalized in a different way

3) PhaseAmpCouplingByAmp it's a different method itself, the method is exactly the name of the function. I think I'll just do I/O improvements

4) bz_Comodulogram is a CFC of amp-amp. there's no duplicates. We either consolidate or re-name it.

5) After all these changes I can consolidate all of them in a function (bz_CrossFreqCoupling or whatever).

Let me know if you guys don't agree with these steps. Thumbs up to endorse these decisions.

AntonioFR8 commented 4 years ago

Hey Eli, This sounds good to me. I remember there was a problem with your MI function in the way it scaled the values. I fixed it in bz_CrossFreqMod. I think you can keep this one and improved as you suggested. For the phase amplitude coupling I think the best normalization is to do a z-score per frequency (you can check my function thetamod.m ) I will keep all these functions separated as they do different stuff and require different inputs.But if you want to also write a wrapper that allows the used to run all of them in the same data. This is what I'm planning to do for the ripple and place cell analysis and I think it could be a good policy in general. Having individual functions for related analysis (e.g. detect ripples, get their metrics, get the spikes and do analysis with them ,etc) and also a master wrapper function that runs all of them sequentially, like a mini ripple (or whatever) analysis pipeline

I can help with checking these functions when you are done

On Wed, Oct 30, 2019 at 7:28 PM Eliezyer notifications@github.com wrote:

Okay, so here is the thing:

1.

bz_CrossFreqMod (Which is not listed in this issue) is a duplicate of bz_ModIndex (phase of multiple low-frequencies against amplitude of multiple high-frequencies). The difference between them is that the first one gives the opportunity to calculate the CFC across many channels, using a fixed channel for the phase and changing the channels for the amplitude. I will keep that functionality and make the function more general (allowing the user to have access to more inputs and etc). 2.

bz_PhaseAmplitudeDist CFC (phase-amp) of a single low-freq(phase) to multiple high-freq(amp). Very similar method to the functions in 1), filtering & wavelet. If we change the way that the phase amplitude map is normalized (sum of the map instead of mean) I can call/use that function from inside the function done in 1). TBH, the CFC from 1) is a duplicate/enhancement of this function but normalized in a different way 3.

PhaseAmpCouplingByAmp it's a different method itself, the method is exactly the name of the function. I think I'll just do I/O improvements 4.

bz_Comodulogram is a CFC of amp-amp. there's no duplicates. We either consolidate or re-name it. 5.

After all these changes I can consolidate all of them in a function (bz_CrossFreqCoupling or whatever).

Let me know if you guys don't agree with these steps. Thumbs up to endorse these decisions.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/buzsakilab/buzcode/issues/292?email_source=notifications&email_token=ADHSQQ47OB7PLWLVT5HACMDQRIKA7A5CNFSM4GY74IKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECWCZ2I#issuecomment-548154601, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADHSQQ3KQDRW7YVOWGWH5J3QRIKA7ANCNFSM4GY74IKA .

-- Antonio Fernandez-Ruiz, PhD Sir Henry Wellcome Post-doctoral fellow Buzsaki Lab. NYU Neuroscience Institute New York University, Langone Medical Center East River Science Park, 450 East 29th Street, 9th Floor New York, NY 10016