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

Migrate kaldi mfcc #681

Merged
merged 4 commits into from Jun 4, 2020
Merged

Migrate kaldi mfcc #681

merged 4 commits into from Jun 4, 2020

Conversation

bhargavkathivarapu
Copy link
Contributor

This PR migrates the kaldi mfcc test (#597 ) from test/test_compliance_kaldi.py to test/kaldi_compatibility_impl.py

Signed-off-by: Bhargav Kathivarapu <bhargavkathivarapu31@gmail.com>
@mthrok
Copy link
Collaborator

mthrok commented Jun 4, 2020

Hi @bhargavkathivarapu

Can you apply this patch?
This should resolve RuntimeError: Expected object of scalar type Double but got scalar type Float for argument #3 'mat2' in call to _th_addmm_out
and the remaining test failures will be all AssertionErrors.

diff --git a/torchaudio/compliance/kaldi.py b/torchaudio/compliance/kaldi.py
index 8115c3a..bdf5f40 100644
--- a/torchaudio/compliance/kaldi.py
+++ b/torchaudio/compliance/kaldi.py
@@ -696,6 +696,8 @@ def mfcc(
     """
     assert num_ceps <= num_mel_bins, 'num_ceps cannot be larger than num_mel_bins: %d vs %d' % (num_ceps, num_mel_bins)

+    device, dtype = waveform.device, waveform.dtype
+
     # The mel_energies should not be squared (use_power=True), not have mean subtracted
     # (subtract_mean=False), and use log (use_log_fbank=True).
     # size (m, num_mel_bins + use_energy)
@@ -717,7 +719,7 @@ def mfcc(
         feature = feature[:, mel_offset:(num_mel_bins + mel_offset)]

     # size (num_mel_bins, num_ceps)
-    dct_matrix = _get_dct_matrix(num_ceps, num_mel_bins)
+    dct_matrix = _get_dct_matrix(num_ceps, num_mel_bins).to(dtype=dtype, device=device)

     # size (m, num_ceps)
     feature = feature.matmul(dct_matrix)
@@ -725,7 +727,7 @@ def mfcc(
     if cepstral_lifter != 0.0:
         # size (1, num_ceps)
         lifter_coeffs = _get_lifter_coeffs(num_ceps, cepstral_lifter).unsqueeze(0)
-        feature *= lifter_coeffs
+        feature *= lifter_coeffs.to(device=device, dtype=dtype)

     # if use_energy then replace the last column for htk_compat == true else first column
     if use_energy:

Signed-off-by: Bhargav Kathivarapu <bhargavkathivarapu31@gmail.com>
Signed-off-by: Bhargav Kathivarapu <bhargavkathivarapu31@gmail.com>
Signed-off-by: Bhargav Kathivarapu <bhargavkathivarapu31@gmail.com>
@codecov
Copy link

codecov bot commented Jun 4, 2020

Codecov Report

Merging #681 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #681   +/-   ##
=======================================
  Coverage   88.72%   88.72%           
=======================================
  Files          22       22           
  Lines        2341     2342    +1     
=======================================
+ Hits         2077     2078    +1     
  Misses        264      264           
Impacted Files Coverage Δ
torchaudio/compliance/kaldi.py 96.06% <100.00%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53e5074...5415881. Read the comment docs.

@bhargavkathivarapu bhargavkathivarapu marked this pull request as ready for review June 4, 2020 06:18
@bhargavkathivarapu
Copy link
Contributor Author

@mthrok , Applied the kaldi mfcc patch . Removed all assertion failing tests

@mthrok
Copy link
Collaborator

mthrok commented Jun 4, 2020

Hi @bhargavkathivarapu

Looks great! Thanks!

@mthrok mthrok merged commit b4284de into pytorch:master Jun 4, 2020
@mthrok
Copy link
Collaborator

mthrok commented Jun 4, 2020

@bhargavkathivarapu

Follow up: test_mfcc_000, test_mfcc_070, test_mfcc_071 are failing on GPU.
https://app.circleci.com/pipelines/github/pytorch/audio/2031/workflows/2fb51661-2656-4d4a-a3d6-b104398254d6/jobs/51900

If you have time, can you remove them too?

@bhargavkathivarapu
Copy link
Contributor Author

Will remove these in next PR for kaldi migration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants