csteinmetz1 / auraloss

Collection of audio-focused loss functions in PyTorch
Apache License 2.0
717 stars 66 forks source link

LinearSTFTLoss #18

Closed turian closed 3 years ago

turian commented 3 years ago

Since SpectralConvergence is not symmetric and a divergence, I add the LinearSTFTLoss.

I also add a small optimization that losses which have weight 0 are not computed.

This seems to run okay in my notebooks

csteinmetz1 commented 3 years ago

Hey @turian, I agree that we should have a STFT loss computed in linear vs. log space. I think this should be implemented by changing the LogSTFTMagnitudeLoss class to be simply STFTMagnitudeLoss, which then has an optional log parameter that can be passed to compute the difference on the log terms.

turian commented 3 years ago

@csteinmetz1 However, it would be nice still to be able to mix log and lin STFT, not just either/or, like in the DDSP paper. Do you agree?

If so would you like me to move forward with your proposed change?

csteinmetz1 commented 3 years ago

Agreed, it would be nice to mix them as you have done. This should be possible in how I propose we change the interface. So you could do something like.

# .... in init
self.logstft = STFTMagnitudeLoss(log=True)
slef.linstft = STFTMagnitudeLoss()

#.... in forward

sc_loss = self.spectralconv(x_mag, y_mag)
log_mag_loss = self.logstft(x_mag, y_mag)
lin_mag_loss = self.linstft(x_mag, y_mag)
...
loss = (self.w_sc * sc_loss) + (self.w_logmag * log_mag_loss) + (self.w_lin_mag * linmag_loss)
turian commented 3 years ago

Done, please look. I haven't run this yet because I have an experiment in progress I don't want to kill.

If you would like to update the docs or README, I encourage you to push to this branch. I have given you committer access to my fork.

turian commented 3 years ago

@csteinmetz1 good to merge? It turns out that in my experiment, l_mag=0.5 l_lin=0.5 works best. I would prefer to be pip install'ing from your master and not my branch.

csteinmetz1 commented 3 years ago

Looks good, will merge now, and I'll update the README and pypi shortly. Thanks for putting this together, should add more flexibility. (Now we just need to get some tests in here...)