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.GriffinLim #1421

Merged
merged 9 commits into from Apr 6, 2021

Conversation

yoyololicon
Copy link
Collaborator

@yoyololicon yoyololicon commented Apr 3, 2021

ref #1414
cc @mthrok

Have incorrect jacobian when doing gradcheck.

@mthrok
Copy link
Collaborator

mthrok commented Apr 5, 2021

Have incorrect jacobian when doing gradcheck.

Hi @yoyololicon

Thanks for the PR. Maybe the reason why it is failing could be the use of float() in the middle of the for loop.

for _ in range(n_iter):
# Store the previous iterate
tprev = rebuilt
# Invert with our current estimate of the phases
inverse = torch.istft(specgram * angles,
n_fft=n_fft,
hop_length=hop_length,
win_length=win_length,
window=window,
length=length).float()

Could you modify the code (just removing .float() is a good start and could be sufficient) so that all the intermediate variables have the same dtype as the input Tensor?

@mthrok mthrok marked this pull request as ready for review April 5, 2021 22:19
@mthrok mthrok marked this pull request as draft April 5, 2021 22:20
@yoyololicon yoyololicon marked this pull request as ready for review April 6, 2021 05:26
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. Thank you!

@mthrok mthrok merged commit 6929331 into pytorch:master Apr 6, 2021
@mthrok mthrok mentioned this pull request Apr 6, 2021
15 tasks
@yoyololicon yoyololicon deleted the griffinlim-autograd-test branch April 6, 2021 15:49
carolineechen pushed a commit to carolineechen/audio that referenced this pull request Apr 30, 2021
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

4 participants