Spectrogram calculation issue with power=1

[ttaoREtw]

🐛 Bug

audio/torchaudio/functional.py

Line 209 in 42a705d

spec_f = spec_f.pow(power).sum(-1) # get power of "complex" tensor

While this is true when power=2,
it is summing up real part and imaginary part when power=1.

[jamarshon]

https://pytorch.org/audio/transforms.html#torchaudio.transforms.Spectrogram
The definition provided here is:

power (int) – Exponent for the magnitude spectrogram, (must be > 0) 
e.g., 1 for energy, 2 for power, etc. (Default: 2)

@ttaoREtw What do you think is the expected behaviour for power=1, power=0, etc.?

[ttaoREtw]

For a complex number c = x+jy, the magnitude of c is |c| = (x^2 + y^2).sqrt().
In your version, however, the result will be x + y.
Thus, it seems to me that for power=1, it should be

if power == 1:
    # for each complex number x + jy, we get  (x^2 + y^2).sqrt()
    spec_f = spec_f.pow(2).sum(-1).sqrt()  

[vincentqb]

So, if power is specified, you'd return the power of the magnitude (aka absolute value) of the complex number via spec_f = spec_f.pow(2).sum(-1).sqrt().pow(power) ?

The code currently behaves very closely to L^p norms: (sum_i |x_i|^p)^{1/p} even for non-integers p >= 0. The difference with the code is the absence of absolute value and the p-th root. I would expect this to have been the original intention.

• For the case p=2, we recover the standard magnitude of a complex number, as you mentioned.
• For p=1, we get |x_0| + |x_1| as mentioned above -- not what the code is doing
• For p=0, we get len((x,y)) -- always 2 for complex numbers -- what the code is doing
• For p=infinity, we get max(|x_i|) -- not currently supported by the code.

Mixing normalization by the L^2 norm (when using normalized) with other power is a little strange.

• I would expect power to be use to to normalize by the p-th norm, when specified (and remove the normalized option if it does not interfere with other places).
• If we want the magnitude, or other p-th norm, I would simply have a separate function to compute them, which the user can explicitly invoke.

Thoughts?

[vincentqb]

I'd say that the code should keep normalized as normalizing by the L2 norm is a fairly standard thing. In addition, I see the following possibilities:

1. Deprecate power which can be misleading. Users can easily derive the current behavior afterwards.
2. Use power also in norm when normalized=True, with default value 2 when normalizing but no power is specified.
3. Add absolute value in spec_f = spec_f.abs().pow(power).sum(-1) and also extend power to any non-negative floats.

I'd vote for 1 unless this breaks things elsewhere. Thoughts? @jamarshon

[ttaoREtw]

I found out that Librosa does the thing as I think.
https://github.com/librosa/librosa/blob/master/librosa/core/spectrum.py#L2524
They use power standing for the power of the magnitude.

[vincentqb]

I see, so librosa does spec_f = spec_f.power(2).sum(-1).sqrt().pow(power) or, using the new syntax, spec_f = complex_norm(spec_f, power=power). We should also check how Kaldi is doing it to make sure we wouldn't break the compliance interface.

[vincentqb]

Closed by #389

[vincentqb]

#250