🔗 Helpful links

• 🧪  See artifacts and rendered test results at hud.pytorch.org/pr/73434
• 📄  Preview Python docs built from this PR
• 📄  Preview C++ docs built from this PR
• ❓Need help or want to give feedback on the CI? Visit our office hours

💊 CI failures summary and remediations

As of commit f2f924a (more details on the Dr. CI page):

@pytorchbot merge this please

Merge failed due to Refusing to merge as mandatory check Lint failed for rule superuser
Raised by https://github.com/pytorch/pytorch/actions/runs/2261168663

@pytorchbot merge this

This PR seems to be causing slight forward-backward compatibility problems (by removing torch.functional.F symbol), that few other projects (Opacus for example) relied on, and also seem to have an incomplete model upgrader logic. But at the same time PR description as super succinct.

@peterbell10 can you please provide a more detailed description to your PRs especially if they introduce something that looks like backward incompatible changes.

@anjali411 please do not accept the PRs with one-line description that introduce non-trivial changes especially if they had to re-landed as introducing some regressions in other ecosystem projects. And if some backward incompatible changes are introduced, make sure that upgrade codepaths are thoroughly tested.

This PR seems to be causing slight forward-backward compatibility problems (by removing torch.functional.F symbol)

You mean someone is explicitly importing torch.functional.F instead of torch.nn.functional? Does that mean every import removal is considered a BC-break?

can you please provide a more detailed description to your PRs especially if they introduce something that looks like backward incompatible changes.

I thought the whole point of the JIT upgraders is that this isn't considered a BC break any more? Or do you mean the import?

Hey @malfet F should have never been exposed in torch.functional so this is also goes back to a bigger discussion of our public API policies. Internal failures were fixed by Natalia (see https://fb.workplace.com/groups/1405155842844877/posts/5840766845950399/?comment_id=5841992042494546&reply_comment_id=5842183229142094)

cc. @ngimel @albanD

You mean someone is explicitly importing torch.functional.F instead of torch.nn.functional? Does that mean every import removal is considered a BC-break?

Yeah. F was being exposed in torch.functional since we don't have an __all__ for torch.functional. So, yeah the import removals would be considered bc breaking if we remove something that we were (intentionally or unintentionally) exposing before.

Hi @anjali411 , there were few more internal projects that dependent on the change, so I have to push some updates as well.
But it looks like upgrader is not configured correctly, as running something as simple as:

$ python -c "import torch;import torchaudio; torch.jit.script(torchaudio.transforms.Spectrogram()).save('foo.pth')"

before the change and the loading the model after results in an error:

% python -c "import torch;print(torch.load('foo.pth'))"                                                            
/Users/nshulga/miniconda3/envs/py_39/lib/python3.9/site-packages/torch/serialization.py:707: UserWarning: 'torch.load' received a zip file that looks like a TorchScript archive dispatching to 'torch.jit.load' (call 'torch.jit.load' directly to silence this warning)
  warnings.warn("'torch.load' received a zip file that looks like a TorchScript archive"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/nshulga/miniconda3/envs/py_39/lib/python3.9/site-packages/torch/serialization.py", line 711, in load
    return torch.jit.load(opened_file)
  File "/Users/nshulga/miniconda3/envs/py_39/lib/python3.9/site-packages/torch/jit/_serialization.py", line 164, in load
    cpp_module = torch._C.import_ir_module_from_buffer(
RuntimeError: 

aten::stft(Tensor self, int n_fft, int? hop_length=None, int? win_length=None, Tensor? window=None, bool center=True, str pad_mode="reflect", bool normalized=False, bool? onesided=None, bool? return_complex=None) -> (Tensor):
Expected a value of type 'str' for argument 'pad_mode' but instead found type 'Optional[bool]'.
:
  File "code/__torch__/torch/functional.py", line 21
  else:
    input0 = input
  _2 = torch.stft(input0, n_fft, hop_length, win_length, window, normalized, onesided, return_complex)
       ~~~~~~~~~~ <--- HERE
  return _2
'stft' is being compiled since it was called from 'spectrogram'
  File "/Users/nshulga/miniconda3/envs/py_3.7-torch-1.11/lib/python3.7/site-packages/torchaudio/functional/functional.py", line 102

    # default values are consistent with librosa.core.spectrum._spectrogram
    spec_f = torch.stft(
             ~~~~~~~~~~ <--- HERE
        input=waveform,
        n_fft=n_fft,
Serialized   File "code/__torch__/torchaudio/functional/functional.py", line 25
  shape = torch.size(waveform0)
  waveform2 = torch.reshape(waveform0, [-1, shape[-1]])
  spec_f = __torch__.torch.functional.stft(waveform2, n_fft, hop_length, win_length, window, center, pad_mode, False, onesided, True, )
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ <--- HERE
  _1 = torch.add(torch.slice(shape, None, -1), torch.slice(torch.size(spec_f), -2))
  spec_f0 = torch.reshape(spec_f, _1)
'spectrogram' is being compiled since it was called from 'Spectrogram.forward'
Serialized   File "code/__torch__/torchaudio/transforms.py", line 18
  def forward(self: __torch__.torchaudio.transforms.Spectrogram,
    waveform: Tensor) -> Tensor:
    _0 = __torch__.torchaudio.functional.functional.spectrogram
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ <--- HERE
    window = self.window
    center = self.center

Which also affects internal pipeline, see https://fb.workplace.com/groups/4571909969591489/posts/5035659103216571/?comment_id=5036153876500427

Can someone revert?

@pytorchbot revert this please

Revert unsuccessful: please retry the command explaining why the revert is necessary, e.g. @pytorchbot revert this as it breaks mac tests on trunk, see {url to logs}.

@pytorchbot revert this please as it breaks backward compatibility of torchaudio models

Will not revert as @peterbell10 is not a MEMBER, but COLLABORATOR

@pytorchbot revert this please as it breaks backward compatibility of torchaudio models

Doing this for Peter above who doesn't have the rights.

FYI This PR also seems to be BC breaking on some torch hub models

Can't reopen this PR, so have opened #77257. Will continue discussion there.