Skip to content

Conversation

@mthrok
Copy link
Contributor

@mthrok mthrok commented Mar 18, 2020

In combination with pytorch/pytorch#34827, I was wrapping up istft function from pytorch package.
I thought all the tests would pass, but after moving istft to pytorch, the function is no longer jit-able as it does not have corresponding C++ implementation in ATen.

@vincentqb Should we disable the test until the C++ implmentation of istft becomes available?

$ python test/test_functional.py
/scratch/moto/torchaudio/torchaudio/functional.py:124: UserWarning: istft has been moved to PyTorch and will be deprecated, please use torch.istft instead.
  warnings.warn(
....../home/moto/conda/envs/PY3.8-cuda101/lib/python3.8/site-packages/librosa/filters.py:235: UserWarning: Empty filters detected in mel frequency basis. Some channels will produce empty responses. Try increasing your sampling rate (and fmax) or reducing n_mels.
  warnings.warn('Empty filters detected in mel frequency basis. '
....................../home/moto/conda/envs/PY3.8-cuda101/lib/python3.8/tempfile.py:819: ResourceWarning: Implicitly cleaning up <TemporaryDirectory '/tmp/tmpmphuzfhl'>
  _warnings.warn(warn_message, ResourceWarning)
.......E...gain: can't reclaim headroom
.
======================================================================
ERROR: test_torchscript_griffinlim (__main__.TestFunctional)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test/test_functional.py", line 74, in test_torchscript_griffinlim
    _test_torchscript_functional(
  File "test/test_functional.py", line 31, in _test_torchscript_functional
    jit_out, py_out = _test_torchscript_functional_shape(py_method, *args, **kwargs)
  File "test/test_functional.py", line 21, in _test_torchscript_functional_shape
    jit_method = torch.jit.script(py_method)
  File "/scratch/moto/pytorch/torch/jit/__init__.py", line 1296, in script
    fn = torch._C._jit_script_compile(qualified_name, ast, _rcb, get_default_args(obj))
  File "/scratch/moto/pytorch/torch/jit/_recursive.py", line 568, in try_compile_fn
    return torch.jit.script(fn, _rcb=rcb)
  File "/scratch/moto/pytorch/torch/jit/__init__.py", line 1296, in script
    fn = torch._C._jit_script_compile(qualified_name, ast, _rcb, get_default_args(obj))
RuntimeError:
Unknown builtin op: aten::istft.
Here are some suggestions:
	aten::stft
	aten::ifft
	aten::irfft

The original call is:
  File "/scratch/moto/torchaudio/torchaudio/functional.py", line 126
    warnings.warn(
        'istft has been moved to PyTorch and will be deprecated, please use torch.istft instead.')
    return torch.istft(
           ~~~~~~~~~~~ <--- HERE
        input=stft_matrix, n_fft=n_fft, hop_length=hop_length, win_length=win_length, window=window,
        center=center, pad_mode=pad_mode, normalized=normalized, onesided=onesided, length=length)
'istft' is being compiled since it was called from 'griffinlim'
  File "/scratch/moto/torchaudio/torchaudio/functional.py", line 248

        # Invert with our current estimate of the phases
        inverse = istft(specgram * angles,
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~...  <--- HERE
                        n_fft=n_fft,
                        hop_length=hop_length,


----------------------------------------------------------------------
Ran 40 tests in 12.376s

FAILED (errors=1)

@mthrok mthrok requested a review from vincentqb March 18, 2020 16:37
@mthrok mthrok closed this Mar 18, 2020
@mthrok mthrok deleted the istft branch March 18, 2020 19:19
@vincentqb
Copy link
Contributor

The test you show uses istft through the pytorch interface. There should be a way of invoking istft in a way that maintains jitability of istft. Have you tried jitting the istft function directly?

Let's make sure all the tests continue passing during the transition.

@mthrok
Copy link
Contributor Author

mthrok commented Mar 19, 2020

import torch

kwargs = {
    'n_fft': 15,
    'hop_length': 3,
    'win_length': 11,
    'window': torch.hamming_window(11),
    'center': True,
    'pad_mode': 'constant',
    'normalized': True,
    'onesided': False,
}
input = torch.stft(torch.randn(3, 15), **kwargs)
jit_istft = torch.jit.script(torch.istft)

py_result = torch.istft(input, **kwargs)
jit_result = jit_istft(input, **kwargs)

print(py_result)
print(jit_result)

assert torch.allclose(py_result, jit_result, atol=1e-6)

I extracted the essence of the test and ran it separately, it turned out istft is jit-able after moving to pytorch.
So I guess there is something wrong with the test. I will take a look into it.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants