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 129 forks source link

Window length in bz_FindRipples.m #418

Open 37ig opened 1 year ago

37ig commented 1 year ago

Hi,

I'm a bit confused about how the RMS window length is computed in bz_FindRipples.m

Lines 132 reads: windowLength = frequency/frequency*11;

Which should always set the windowLength to 11 data-points. Is this as intended or an error?

Thanks for any help!

brendonw1 commented 1 year ago

That is strange. That line is very odd and may even always result in a value of 11 (? depending on order of operations). I am cc'ing Antonio and David in case they might know more. I've almost never used that code and definitely didn't significantly write any of it

Brendon

Brendon Watson, MD-PhD Assistant Professor in Psychiatry Biomedical Sciences Research Building, Room 5059 University of Michigan 109 Zina Pitcher Place Ann Arbor, MI 48109-5720 Lab Website: http://watsonneurolab.org Clinical phone: 734-764-0231 http://voice.google.com/calls?a=nc,%2B17347640231

On Wed, Dec 28, 2022 at 4:12 PM 37ig @.***> wrote:

Hi,

I'm a bit confused about how the RMS window length is computed in bz_FindRipples.m

Lines 132 reads: windowLength = frequency/frequency*11;

Which should always set the windowLength to 11 data-points. Is this as intended or an error?

Thanks for any help!

— Reply to this email directly, view it on GitHub https://github.com/buzsakilab/buzcode/issues/418, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA26WTNFEQMF74SD22EPCM3WPSUIVANCNFSM6AAAAAATLSBNF4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

37ig commented 1 year ago

Yes the order of operations means it does return 11. With a bit more investigation it looks like the bz_FindRipples.m may have been inspired by (or vice versa) the FMAToolbox function FindRipples.m.

The line in the FMAToolbox function is windowLength = round(frequency/1250*11); which makes much more sense. But would be useful if someone could confirm this please.

Cheers!

dlevenstein commented 1 year ago

This should absolutely be

windowLength = round(frequency/1250*11);

not sure when it got changed, or who's using that code rn - anyone?

dlevenstein commented 1 year ago

actually - I bet what happened is someone wanted to change the 1250 to be the sampling frequency from the sessionInfo, and then got messed up between the frequency of the filter vs the sampling frequency....

evilrobotxoxo commented 1 year ago

IMO this should be written as

round(11 * frequency/1250)

because under PEMDAS frequency/125011 is equal to frequency/(125011), but under BODMAS it's equal to (frequency/1250)*11

We should avoid that sort of ambiguity.

On Fri, Jan 6, 2023 at 1:19 PM Dan Levenstein @.***> wrote:

actually - I bet what happened is someone wanted to change the 1250 to be the sampling frequency from the sessionInfo, and then got messed up between the frequency of the filter vs the sampling frequency....

— Reply to this email directly, view it on GitHub https://github.com/buzsakilab/buzcode/issues/418#issuecomment-1373979317, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABG4WJN56ITK4Q3HAI34S4DWRBO2HANCNFSM6AAAAAATLSBNF4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>