Potential mistake in the documentation of calculating time constant based on knee frequency

[duodenum96]

Hello,

There is potentially a mistake in the documentation for calculating time constants. The code shown there:

time_constant = compute_time_constant(fm.get_params('aperiodic', 'knee'))

doesn't take exponents different than 1 into account since compute_time_constant is just doing 1/(2 pi knee_frequency). So the user should provide not just the parameter for knee but actual knee frequency as calculated in the previous code block in the same documentation. It works fine in the example because the provided exponent is 1 in the simulated spectrum:

freqs2, powers2 = gen_power_spectrum([1, 50], [0, 15, 1], [8, 0.125, 0.75], freq_res=0.25)

Here is a complete example:

import fooof
from fooof import FOOOF
from fooof.utils.params import compute_time_constant, compute_knee_frequency
from fooof.sim import gen_power_spectrum

offset = 0
knee = 15
exponent = 2
freqs2, powers2 = gen_power_spectrum([1, 50], [offset, knee, exponent], [8, 0.125, 0.75], freq_res=0.25)
fm = FOOOF(min_peak_height=0.05, aperiodic_mode='knee')
fm.fit(freqs2, powers2)

knee_frequency = fooof.utils.params.compute_knee_frequency(*fm.aperiodic_params_[1:])
param_knee = fm.get_params("aperiodic", "knee")
print(f"Knee Frequency: {knee_frequency}")
print(f"get_params result: {param_knee}")
time_constant_1 = fooof.utils.params.compute_time_constant(knee_frequency)
time_constant_2 = fooof.utils.params.compute_time_constant(param_knee)
print(f"Time Constant via compute_time_constant(knee_frequency): {time_constant_1}")
print(f"Time Constant via compute_time_constant(get_params()): {time_constant_2}")

which returns:

Knee Frequency: 3.8880645658967623
get_params result: 15.191039323680608
Time Constant via compute_time_constant(knee_frequency): 0.04093423357417601
Time Constant via compute_time_constant(get_params()): 0.010476896261060695

I can open a PR if you agree btw. Thanks for the amazing package!

[voytek]

Oh! I do believe you're correct.

@rdgao, confirm please?

@duodenum96, once Richard confirms, please open a PR and I'll review and approve, and you will have officially contributed to specparam! Thanks for catching this.

[rdgao]

Thanks for digging into the code @duodenum96! Just to make sure we're on the same page, you're saying that in the documentation:

time_constant = compute_time_constant(fm.get_params('aperiodic', 'knee'))

should not take as an input arg the knee "parameter" as returned by get_params(), but the knee frequency which is computed with the additional step fooof.utils.params.compute_knee_frequency(*fm.aperiodic_params_[1:])?

I think you are right, and the easiest fix would be to correct the documentation, and not the function. That's what you're proposing right?

[duodenum96]

Yep, you are right. No need to change the actual function, just a quick fix in the docs.

[rdgao]

sounds good!