Closed simonschwaer closed 9 months ago
Thanks @simonschwaer ! I would love to merge these changes. I think everything is good that doesn't cause any change in default behavior. So, would it be possible to split change 2. dealing with eps
to a separate PR?
Thanks @simonschwaer ! I would love to merge these changes. I think everything is good that doesn't cause any change in default behavior. So, would it be possible to split change 2. dealing with
eps
to a separate PR?
Done.
LGTM
Here are a few changes I made to make the library even more useful for me... I can remove some commits or split them into separated PRs if desired.
1. Add option to use any window function available in scipy.signal
This allows to use additional window types for
STFTLoss
by providing a window type implemented inscipy.signal.windows.get_window()
(docs) – or at least the ones that do not need additional arguments. The torch windows are still supported, so it does not change the default behaviour or other existing code.2. Change magnitude calculation in STFTLoss for better interpretability of the eps parameter
This would change the default behaviour, since previously, all values were clamped at $10^{-4}$ by default as
torch.clamp
is applied insidetorch.sqrt
, resulting in a threshold of $\sqrt{10^{-8}}$. I think it would be more intuitive if theeps
specified inSTFTLoss
would actually specify the resulting smallest value.3. Allow for more flexible log compression
By using $\log(\gamma x + \varepsilon)$ for the log-compression in
STFTMagnitudeLoss
, the amount of compression and output value range becomes more controllable. For example, $\varepsilon = 1$ would lead to a minimum output value of $0$, and $\gamma = 10$ would compress the magnitudes more. With the given default values, the commit does not change the previous default behaviour.4. Compute torch.angle of STFT only when needed
I noticed that the MSS loss was slower than expected on my M1 GPU and CPU. The torch profiler showed that
torch.angle
is taking a lot of time, even though it is not used in many cases. The proposed improvement just calculates the angle only if it is actually used for the loss.With
torch.angle
:Without
torch.angle
: