csteinmetz1 / auraloss

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

Butterworth filtering #64

Closed jmoso13 closed 1 year ago

jmoso13 commented 1 year ago

Adding capability to use custom filters via scipy butter filter in lieu of perceptual weighting. Added kwargs:

Added filter_ntaps as a variable kwarg to increase fidelity of FIR Filter.

Also added comments explaining new kwargs. Have tested models using these updated filtered loss functions and looks to be working as expected without changing existing functionality. Eventually might be nice to replace perceptual_weighting and filter_weighting bool kwargs with a unified kwarg to switch between any of them but didn't want to mess up current use.

csteinmetz1 commented 1 year ago

Thanks for adding this functionality!

I am thinking on other ways to add the butterworth filtering to the main STFT class without adding more arguments. One option would be to allow a single FIRFillter object as an argument (optional) and then the user would construct this filter outside the class and it would be applied in the forward. What do you think of that?

jmoso13 commented 1 year ago

I like that option a lot, would require people feeling comfortable with importing/constructing the filter on its own but honestly I've already been doing this quite easily to add filters to other loss functions like L1 time loss in my models so I think that would be a great option. I can draft a PR with that instead.

Is current perceptual_weighting functionality already tagged in a version? Would it be cool if I removed that while adding the FIRFilter arg? Don't want to mess up current use.

csteinmetz1 commented 1 year ago

Let's keep perceptual_weighting for now. We can maintain this by internally creating the FIRFilter object if that flag is set, and keep the new FIRFilter parameter as a separate filter that is also applied.

jmoso13 commented 1 year ago

closing this PR to open a new one with the approach outlined above