Errors (IndexError) on silent waves (degenerate pathological cases)

[nyanpasu64]

  File "/home/nyanpasu64/miniconda3/envs/wavetable/lib/python3.6/site-packages/waveform_analysis/freq_estimation.py", line 92, in freq_from_autocorr
    start = find(d > 0)[0]
IndexError: index 0 is out of bounds for axis 0 with size 0

If freq_from_autocorr is given a silent signal d contains a series of 0.

I feel the best solution is to just return some arbitrary value without crashing.

---

I don't remember if it happens in practice, but a while back I found that parabolic(f, 0) compares to f[-1] (which is wrong but doesn't crash) and parabolic(f, len(f)-1) compares to f[len(x)] which raises IndexError.

[endolith]

I'm not sure if the frequency estimation function should handle this or not. If it should, it should probably return None, but maybe it should be the calling function's responsibility to only feed it signals with a frequency, or use try .. except to catch the errors when it doesn't.

[nyanpasu64]

Idea: parabolic(f, x)

# if x < 0 or x >= len(f): raise IndexError(add meaningful message?)
if x <= 0 or x >= len(f) - 1:
    return (x, f[x]) # generates IndexError for x >= len(f), but not x < 0

Also, since freq_from_autocorr performs DC removal, I feel it's unacceptable (for my use cases) for a utility function to crash given any DC input, due to an unhandled internal IndexError. I think to make the function easier to integrate into surrounding code, it should provide a result (f=0?) with the same type as normal execution, or maybe raising ValueError (though I still don't like the idea of a frequency estimator crashing and taking down the entire app, given hard-to-estimate input where I probably don't care about the frequency).

[endolith]

@nyanpasu64

In what context are you using it that it's unacceptable to raise an exception? A silent WAV file has no frequency, so I'm not sure what you would expect it to do here. It certainly shouldn't return an arbitrary value in Hz.

It would make some sense to return 0, since it's just a DC signal at 0 Hz.

It could also return None or NaN.

[nyanpasu64]

It's been years since I've worked with this library (and recently I tried getting some of my code to run again, but waveform-analysis fails from importing scipy.signal.kaiser which has been moved to scipy.signal.windows.kaiser) (EDIT: fixed in 10b5781302dbd81a23393b7daa516598ecdc85f3). I'm no longer familiar with the code involved here. Looking back at my old code it appears I catch IndexError and give it a value of 0 instead. That might be a good idea?

In what context are you using it that it's unacceptable to raise an exception?

I think leaking an exception from an implementation detail is a bit messy compared to an intentional exception or return value, and having the caller handle an exception might be slower than branching on the return value.

[endolith]

I think leaking an exception from an implementation detail is a bit messy compared to an intentional exception or return value,

Yeah I agree. Just not sure which to do.

https://github.com/endolith/waveform-analysis/discussions/35