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

Add autograd test for T.AmplitudeToDB #1447

Merged
merged 4 commits into from Apr 13, 2021

Conversation

krishnakalyan3
Copy link
Contributor

Ref #1414
cc @mthrok

Copy link
Collaborator

@mthrok mthrok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @krishnakalyan3

Thanks for working on this. Please refer to my comment.

sample_rate = 8000
transform = T.AmplitudeToDB()
waveform = get_whitenoise(sample_rate=sample_rate, duration=0.05, n_channels=2)
self.assert_grad(transform, [waveform], nondet_tol=1e-10)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try it without nonfat_tol?
With top_db=None, it's just a combination of log10, clamp and multiplications.
If any of them are non deterministic, would be nice to add a comment about it here.

Also can you parameterize top_db? Say, None and 80 as recommended by the doc.

if top_db is not None:

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@krishnakalyan3

My bad, I realized that when top_db is provided the function uses torch.amax, which is not differentiable. I think we should only test for the case top_db=None.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just made those changes let me know if this is okay.

@mthrok mthrok mentioned this pull request Apr 13, 2021
15 tasks
Copy link
Collaborator

@mthrok mthrok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks!

@mthrok mthrok merged commit 931555c into pytorch:master Apr 13, 2021
@krishnakalyan3 krishnakalyan3 deleted the autograd-test-amptodb branch April 14, 2021 06:40
carolineechen pushed a commit to carolineechen/audio that referenced this pull request Apr 30, 2021
mthrok pushed a commit to mthrok/audio that referenced this pull request Dec 13, 2022
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

3 participants