search

keunwoochoi / torchaudio-contrib

A test bed for updates and new features | pytorch/audio
169 stars 22 forks source link

Tests for functionals #66

Closed ksanjeevan closed 5 years ago

ksanjeevan commented 5 years ago

From discussion in the pytorch/audio slack channel, seems like we should be doing testing for functionals.

(Also fixed a bug on the phase_vocoder on gpu)

keunwoochoi commented 5 years ago

Moving lots of things to test_functionals.py makes sense, but maybe not completely? There should be some test for initiating and using Layers, too. Otherwise, things look good.

ksanjeevan commented 5 years ago

@keunwoochoi how about test_functional for testing shapes and/or values, and test_layers for proper initialization? Maybe test_layers could chain some of the modules too (like spectrogram, a stretched melspectrogram, etc.)?

In any case, we'll go with just test_functional for Phase 1.

keunwoochoi commented 5 years ago

Agreed for all.

On Jun 14, 2019, at 20:05, Kiran Sanjeevan notifications@github.com wrote:

@keunwoochoi how about test_functional for testing shapes and/or values, and test_layers for proper initialization? Maybe test_layers could chain some of the modules too (like spectrogram, a stretched melspectrogram, etc.)?

In any case, we'll go with just test_functional for Phase 1.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

keunwoochoi commented 5 years ago

Hey @ksanjeevan, I think it's good to be merged. If all seems good please go ahead (and maybe make a PR to the official repo, from some point that repo would be a better place to communicate with their maintainers).

ksanjeevan commented 5 years ago

@keunwoochoi - Actually I'm almost done doing some changes with the proposed changes mentioned:

how about test_functional for testing shapes and/or values, and test_layers for proper initialization? Maybe test_layers could chain some of the modules too (like spectrogram, a stretched melspectrogram, etc.)?

Just sorting a couple of bugs and I'll update this PR. Once that is done, if it looks good, 👍 to go ahead with the PR to the official repo (will update them on slack).

ksanjeevan commented 5 years ago

So I've checked some basic stuff for modules that created parameters. Maybe we want to check more? I think this makes sense. Also added tests for two combinations of modules: SpectrogramDb (just chaining spectrogram with a db conversion) and MelspectrogramStretch (A melspectrogram that's been stretched).

If these don't look good that's fine we can put them on hold, as long as we all agree on test_functional so we can proceed with Phase 1!

keunwoochoi commented 5 years ago

Hey, all look good. I'm not sure if we're testing enough but that's what torch/audio reviewers' would have opinions. Let's ship it :)