Add autograd test for T.Spectrogram/T.MelSpectrogram #1340

mthrok · 2021-03-02T21:54:53Z

Add test for checking auto grad. Part of #1337

test/torchaudio_unittest/autograd_test_impl.py

anjali411 · 2021-03-02T23:27:18Z

test/torchaudio_unittest/autograd_cuda_test.py

+
+
+@skipIfNoCuda
+class AutogradCUDATest(AutogradTestCase, PytorchTestCase):


I think it would be nice to have a device generic class similar to what we have in PyTorch for autograd tests https://github.com/pytorch/pytorch/blob/a3a2150409472fe6fa66f3abe9e795303786252c/test/test_autograd.py#L7931-L7937

Sure, but let's defer on changing the test infrastructure. This is the common pattern used in torchaudio, so if we are going to change, I would like to change them in one go.

mthrok · 2021-03-03T18:24:58Z

@anjali411 I have also added T.MelSpectrogram. let me know what you think.

test/torchaudio_unittest/transforms/autograd_test_impl.py

anjali411 · 2021-03-03T20:14:51Z

test/torchaudio_unittest/transforms/autograd_test_impl.py

+        for i in inputs:
+            i.requires_grad = True
+            inputs_.append(i.to(dtype=self.dtype, device=self.device))
+        assert gradcheck(transform, inputs_, eps=eps, atol=atol, rtol=rtol)


self.assertTrue(gradcheck(transform, inputs_))

anjali411 · 2021-03-03T20:15:46Z

test/torchaudio_unittest/transforms/autograd_test_impl.py

+
+
+class AutogradTestCase(TestBaseMixin):
+    def assert_grad(self, transform, *inputs, eps=1e-06, atol=1e-05, rtol=0.001):


you don't need to define these values eps, atol, rtol here

anjali411 · 2021-03-03T20:16:03Z

test/torchaudio_unittest/transforms/autograd_test_impl.py

+            i.requires_grad = True
+            inputs_.append(i.to(dtype=self.dtype, device=self.device))
+        assert gradcheck(transform, inputs_, eps=eps, atol=atol, rtol=rtol)
+        assert gradgradcheck(transform, inputs_, eps=eps, atol=atol, rtol=rtol)


`self.assertTrue(gradgradcheck(transform, inputs_))

test/torchaudio_unittest/transforms/autograd_test_impl.py

albanD · 2021-03-03T20:18:05Z

test/torchaudio_unittest/transforms/autograd_test_impl.py

+        for i in inputs:
+            i.requires_grad = True
+            inputs_.append(i.to(dtype=self.dtype, device=self.device))
+        assert gradcheck(transform, inputs_, eps=eps, atol=atol, rtol=rtol)


nit: use self.assertTrue()

Is there a reason that assertTrue is preferred?
I always think that the message returned by assertTrue, AssertionError: False is not true non sense, so I always use assert.

I was mostly suggesting to use the self. variant of the assert instead of the default one.
The reason is that it makes the test framework aware of it in a better way than catching the error from the assert. And it allows it to give better messages when running the whole test suite.

test/torchaudio_unittest/transforms/autograd_test_impl.py

mthrok · 2021-03-04T01:05:39Z

@albanD @anjali411

What does this mean? How much should we be worried?

RuntimeError: Backward is not reentrant, i.e., running backward with same input and grad_output multiple times gives different values, although analytical gradient matches numerical gradient. The tolerance for nondeterminism was 0.0.

https://app.circleci.com/pipelines/github/pytorch/audio/5301/workflows/782e2d49-6f99-4e98-a240-b6597557d855/jobs/176159

albanD · 2021-03-04T15:43:37Z

That means that your backward function is not deterministic.
If you expect it to be deterministic, then you might want to double check.
If this is expected, then you can increase the threshold by passing nondet_tol: float to gradcheck with the difference that you expect to see there (this defaults to 0.0). And you should document that the backward is not deterministic (at least that's what we do in core here https://pytorch.org/docs/stable/generated/torch.set_deterministic.html#torch.set_deterministic)

mthrok · 2021-03-04T17:16:34Z

That means that your backward function is not deterministic.
If you expect it to be deterministic, then you might want to double check.
If this is expected, then you can increase the threshold by passing nondet_tol: float to gradcheck with the difference that you expect to see there (this defaults to 0.0). And you should document that the backward is not deterministic (at least that's what we do in core here https://pytorch.org/docs/stable/generated/torch.set_deterministic.html#torch.set_deterministic)

Thanks for the info. This is an interesting finding. Let me dig into that.

mthrok · 2021-03-17T20:38:19Z

@albanD Following the finding in pytorch/pytorch#54093, I added nondet_tol=1e-10. Do you think this is tight enough?

albanD · 2021-03-18T03:04:44Z

Interesting investigation!
In that case yes 1e-10 should be good enough!

anjali411 · 2021-03-29T23:23:43Z

test/torchaudio_unittest/transforms/autograd_test_impl.py

+        self.assert_grad(transform, [waveform], nondet_tol=1e-10)
+
+    def test_melspectrogram(self):
+        # replication_pad1d_backward_cuda is not deteministic and


nit - is nondeterministic

anjali411

LGTM!

mthrok · 2021-03-31T01:10:38Z

Thanks!

Co-authored-by: Brian Johnson <brianjo@fb.com>

facebook-github-bot added the CLA Signed label Mar 2, 2021

mthrok assigned anjali411 Mar 2, 2021

mthrok mentioned this pull request Mar 2, 2021

[Migration] Torchaudio Complex Tensor Support and Migration #1337

Closed

21 tasks

mthrok requested a review from anjali411 March 2, 2021 22:08

mthrok unassigned anjali411 Mar 2, 2021

anjali411 reviewed Mar 2, 2021

View reviewed changes

test/torchaudio_unittest/autograd_test_impl.py Outdated Show resolved Hide resolved

anjali411 reviewed Mar 2, 2021

View reviewed changes

mthrok force-pushed the autograd-test-spectrogram branch from 7454333 to b3fbe41 Compare March 3, 2021 18:23

mthrok changed the title ~~Add autograd test for T.Spectrogram~~ Add autograd test for T.Spectrogram/T.MelSpectrogram Mar 3, 2021

anjali411 reviewed Mar 3, 2021

View reviewed changes

test/torchaudio_unittest/transforms/autograd_test_impl.py Outdated Show resolved Hide resolved

anjali411 reviewed Mar 3, 2021

View reviewed changes

albanD reviewed Mar 3, 2021

View reviewed changes

mthrok added the complex dtype migration label Mar 6, 2021

mthrok mentioned this pull request Mar 16, 2021

stft + abs is non-deterministic in backward path pytorch/pytorch#54093

Closed

mthrok force-pushed the autograd-test-spectrogram branch from b67b2c3 to ba61c9b Compare March 16, 2021 21:27

mthrok added 4 commits March 17, 2021 16:02

Add autograd test for T.Spectrogram

e2fd9b8

Add melspectrogram

863365b

Address most of feedbacks

44efc8c

Add nondet_tol for spectrogram

0cb1e3d

mthrok force-pushed the autograd-test-spectrogram branch from ba61c9b to 0cb1e3d Compare March 17, 2021 16:53

anjali411 reviewed Mar 29, 2021

View reviewed changes

anjali411 approved these changes Mar 29, 2021

View reviewed changes

mthrok merged commit e4a0bd2 into pytorch:master Mar 31, 2021

mthrok deleted the autograd-test-spectrogram branch March 31, 2021 01:10

mthrok mentioned this pull request Apr 2, 2021

Autograd tests for Transforms #1414

Closed

15 tasks

mthrok modified the milestones: Complex Tensor Migration, v0.9 Apr 5, 2021

mthrok pushed a commit to mthrok/audio that referenced this pull request Dec 13, 2022

added 2 mobile cards for ios and android tutorials (pytorch#1340)

7eb9114

Co-authored-by: Brian Johnson <brianjo@fb.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add autograd test for T.Spectrogram/T.MelSpectrogram #1340

Add autograd test for T.Spectrogram/T.MelSpectrogram #1340

mthrok commented Mar 2, 2021

anjali411 Mar 2, 2021

mthrok Mar 3, 2021 •

edited

mthrok commented Mar 3, 2021

anjali411 Mar 3, 2021

anjali411 Mar 3, 2021

anjali411 Mar 3, 2021

albanD Mar 3, 2021

mthrok Mar 3, 2021

albanD Mar 3, 2021

mthrok commented Mar 4, 2021

albanD commented Mar 4, 2021

mthrok commented Mar 4, 2021

mthrok commented Mar 17, 2021

albanD commented Mar 18, 2021

anjali411 Mar 29, 2021

anjali411 left a comment

mthrok commented Mar 31, 2021



		@skipIfNoCuda
		class AutogradCUDATest(AutogradTestCase, PytorchTestCase):



		class AutogradTestCase(TestBaseMixin):
		def assert_grad(self, transform, *inputs, eps=1e-06, atol=1e-05, rtol=0.001):

Add autograd test for T.Spectrogram/T.MelSpectrogram #1340

Add autograd test for T.Spectrogram/T.MelSpectrogram #1340

Conversation

mthrok commented Mar 2, 2021

Choose a reason for hiding this comment

mthrok Mar 3, 2021 • edited

Choose a reason for hiding this comment

mthrok commented Mar 3, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mthrok commented Mar 4, 2021

albanD commented Mar 4, 2021

mthrok commented Mar 4, 2021

mthrok commented Mar 17, 2021

albanD commented Mar 18, 2021

Choose a reason for hiding this comment

anjali411 left a comment

Choose a reason for hiding this comment

mthrok commented Mar 31, 2021

mthrok Mar 3, 2021 •

edited