Confusing naming for argument `n_stft` in MelScale
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 2.9k
- Forks
- 799
- Avg merge
- 58m
- Merged PRs (30d)
- 3
Description
🐛 Describe the bug
The name and docstring of the argument n_stft in MelScale is confusing, because:
- Just seeing the name, it looks equivalent to
n_fft(just changes from FFT to STFT), which is a typical name used for the FFT size. Actually it is used with that purpose in torchaudio Spectrogram and librosa.feature.melspectrogram. - Moreover, the docstring for
n_stftexplicitly states "See n_fft in :class:Spectrogram", which looks as a confirmation thatn_stftis indeed the FFT size.
IMO, a more appropriate name for this argument would be something like n_freqs. Actually, it is under that name in torchaudio.functional.melscale_fbanks
Versions
torch 2.0.0+cu117
torchaudio 2.0.1+cu117
(I think nothing else is relevant)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with src/torchaudio/transforms/_transforms.py, reading the MelScale argument and docstring cited in the issue, then compare the n_freqs naming in src/torchaudio/functional/functional.py at melscale_fbanks. Check related tests and call sites to understand the naming impact; done means the argument documentation and naming clearly distinguish frequency-bin count from FFT size.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, pytorch
- Domain
- data
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100