search

asteroid-team / torch-audiomentations

Fast audio data augmentation in PyTorch. Inspired by audiomentations. Useful for deep learning.
MIT License
971 stars 88 forks source link

SNR & gain distribution #14

Open iver56 opened 4 years ago

iver56 commented 4 years ago

https://github.com/asteroid-team/torch-audiomentations/pull/13#issuecomment-701253966

What are the scenarios that we want to cover?

mpariente commented 4 years ago

As said in the thread, I'd vote for supporting the use of eval to enable the user to choose his distribution.

Copied from the PR 

# Easy normal way
ApplyBackgroundNoise(path, snr=-3)  
# Choose your distribution
ApplyBackgroundNoise(path, snr="np.random.randn()")
# Maybe this as well for uniform? Don't know
ApplyBackgroundNoise(path, snr=[-6, 3])

Picking a fixed distribution is probably too simplistic, and trying to support all the distributions like in scaper seems too heavy.

This won't only apply to SNR, but to speed perturbation as well for example.

iver56 commented 4 years ago

I see. It doesn't seem to difficult to implement. However, in the "Choose your distribution" case, I would rather pass a callable (e.g. a function or a lambda) than a string.

mpariente commented 4 years ago

It could also be supported I guess. The main advantage of a string is that you can easily put it in config files etc.. and keep you augmentation pipeline reproducible without too much trouble.

iver56 commented 4 years ago

Ok. I might be a bit on the cautious side here. I generally avoid eval because of its security implications [1], and because there is often a better way [2] to do it. A callable along with arguments can still be serialized to a config file (one string for the function name and n variables for the arguments), albeit it would indeed be a little bit more work.

[1] https://nedbatchelder.com/blog/201206/eval_really_is_dangerous.html
[2] https://stackoverflow.com/a/1832957/2319697

mpariente commented 4 years ago

Didn't know about the security issue with eval, thanks. It would be more work for us, but also for the users, which we don't want.

On the other side, if you try several distributions for your augmentations and want to compare some metrics in function of it, having a string per condition is less useful than all the arguments. Well IDK.