Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bring back stft test, and remove version limit of librosa #1195

Merged
merged 1 commit into from
May 24, 2023

Conversation

TomonobuTsujikawa
Copy link
Contributor

This PR is to resolve stft/istft test failures, skipped by #1193 .

Start from librosa 0.10.0, its stft implementation introduced some changes that may break existing testing cases of nnabla.
There seems to be an ongoing discussion on STFT inconsistencies regarding tf.signal.stft, torch.stft, librosa.stft and scipy.signal.stft.
But in this PR, we slightly changed one of our stft test parameter that could workaround the problem.

By the way, nnabla test case passes window_type=rectangular directly to librosa.stft, but according to this document:

window: string, tuple, number, function, or np.ndarray [shape=(n_fft,)]
Either:
a window specification (string, tuple, or number); see scipy.signal.get_window
a window function, such as scipy.signal.windows.hann
a vector or array of length n_fft
Defaults to a raised cosine window (‘hann’), which is adequate for most applications in audio signal processing.

And, refered to scipy document, rectangular should be boxcar, so it's also changed in this PR.

@TomonobuTsujikawa
Copy link
Contributor Author

TomonobuTsujikawa commented May 30, 2023

This PR solves this issue again: sony/nnabla-ext-cuda#413

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note-bugfix Auto-release; Bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants