OverLordGoldDragon / ssqueezepy

Synchrosqueezing, wavelet transforms, and time-frequency analysis in Python
MIT License
618 stars 96 forks source link

Doc extract_ridges returns #52

Closed Bulbille closed 2 years ago

Bulbille commented 2 years ago

The doc for extract_ridges

# Returns
        ridge_idxs: np.ndarray
            Indices for maximum frequency ridge(s).
        fridge: np.ndarray
            Frequencies tracking maximum frequency ridge(s).
        max_energy: np.ndarray [n_timeshifts x n_ridges]
            Energy maxima vectors along time axis.

Though in the code, fridge are the scales. (line 137). Please clarify the doc (or change the doc), since fridge has the same dimension as scales. It feels like one of them should change.

Also it feels weird that the following code does not give the expected scales

Twx, Wx, ssq_freqs,scales = ssq_cwt(x,fs=fs,scales='log',maprange='peak',nv=32,wavelet='morlet')
ridge_idxs,fridge,max_energy = extract_ridges(Wx, scales, penalty=10, n_ridges=2, bw=5,get_params=True,transform='cwt')
np.allclose(np.exp(fridge),np.squeeze(scales[ridge_idxs])) # Gives True, shall not be np.exp

In the first line, scales is logarithmic. Using transform='cwt', the outputed scales from extract_ridges are the log of ssq_cwt scales, which are already in log. There's some confusion there.

Third thing : It feels like scales and ssq_freqs arrays are reversed, and to retrieve the right frequencies, I should use the following snipet

Twx, Wx, ssq_freqs,scales = ssq_cwt(x,fs=fs,scales='log',maprange='peak',nv=64,wavelet='morlet')
ridge_idxs,fridge,max_energy = extract_ridges(Twx,ssq_freqs[::-1], penalty=10, n_ridges=2, bw=8,get_params=True,transform='linear')

With the use of the [::-1] to reverse ssq_freqs array., which originates from lines 229 and 230 of ssqueezing.py. Tell me if that is the intended functionnality.

OverLordGoldDragon commented 2 years ago

Thanks for the report.

  1. Fair, will change
  2. I think the purpose is to linearize the scaling, as we want distance between adjacent rows to be uniform so that a fixed bw is applicable.
  3. Fair, I've been considering this - may change in a future release.

@DavidBondesson, I think fridges should return the original scales rather than log of - agreed? And am I right about 2?

OverLordGoldDragon commented 2 years ago

Addressed in latest release.

DavidBondesson commented 2 years ago

Thanks for the report.

1. Fair, will change

2. I think the purpose is to linearize the scaling, as we want distance between adjacent rows to be uniform so that a fixed `bw` is applicable.

3. Fair, I've been considering this - may change in a future release.

@DavidBondesson, I think fridges should return the original scales rather than log of - agreed? And am I right about 2?

@OverLordGoldDragon, To your question: I agree, Thank you for taking care of it!