Add contrast to functional

[bhargavkathivarapu]

Hi ,

Regarding the issue #260 ( Reducing dependency in Sox)
Implemented below function in functional.py

• contrast

Added a test case to test_sox_compatibility.py

All tests in test_sox_compatibility.py passed on CPU.

................gain: can't reclaim headroom
.
----------------------------------------------------------------------
Ran 17 tests in 501.774s

OK

@vincentqb could you please review changes

[vincentqb]

Thanks for working on this! Can you add the following?

• documentation
• torchscript test
• batching test
• sox test

[mthrok]

@bhargavkathivarapu If you need help in adding the tests, let me know.
For batch test you can add test_contrast under TestFunctional and call _test_batch function.
For torchscript test, you can add test_contrast under _FunctionalTestMixin and define an inner function that takes input Tensor as the only argument (other parameters should be embedded in the function), then call self._assert_consistency with input Tensor.

[bhargavkathivarapu]

@vincentqb

• Added batching test for contrast
• Added torchscript test for contrast
• Updated functional.rst for contrast and other missing functions

@mthrok , @vincentqb
I think we should have a CONTRIBUTING.md just like the pytorch repo.
Detailing orgainization of torchaudio code base and with the checklists indicating what tests to do and where to add them when adding new features to functional , transforms or datasets

[mthrok]

Can you use simple random tensor of value range [-1.0, 1.0] i.e. torch.rand(2, 100) - 0.5 ?

The reason is that we do not want this unit test to rely on other library function torchaudio.load.
That way when torchaudio.load is broken, this test is not affected.
I know that test_detect_pitch_frequency does this but it's only because of recent refactoring and in my opinion it should not.

[bhargavkathivarapu]

@mthrok I have this modified .
Are torchaudio functionals conditioned on normalization ?

• Because sox in some functions it internally clips the values as per max and min of input.
• I assumed normalized waveform in writing contrast function .
• In tests also i didn't see anywhere normalization=False.
• If unnormalized waveform is passed, the outputs will differ between existing SoxEffects and functional contrast

[mthrok]

@bhargavkathivarapu

I am not sure if there is/was a design principle for audio normalization but since torchaudio.load has normalization=True by default, and all the tests are written in normalization=True so I think that's de-facto standard.

Talking about batch consistency test here specifically, the purpose of the test is to assure that function returns the same result regardless of batching and not to assure the result is same as SoX. So it's okay to put whatever the tensor, but to be closer to the actual situation, I thought rather than torch.rand (which produces [0, 1]), torch.rand - 0.5 would be slightly better. You can normalize it to be further closer if you would like.

[vincentqb]

I am not sure if there is/was a design principle for audio normalization but since torchaudio.load has normalization=True by default, and all the tests are written in normalization=True so I think that's de-facto standard.

Yes, we assume that waveforms are in the range [-1, 1] in torchaudio.

We should consider documenting this convention in the readme, and guaranteeing it in some form. A caveat is that, when doing a transformation that makes the waveform fall outside (e.g. gain), there may be more than one way to bring the signal back to the [-1, 1] range.

[mthrok]

I think we should have a CONTRIBUTING.md just like the pytorch repo.
Detailing orgainization of torchaudio code base and with the checklists indicating what tests to do and where to add them when adding new features to functional , transforms or datasets

I agree. Recently I also had difficult time figuring out what tests need to be added so I reorganized most of the test, and I was planning to add overview of the test structure. Thanks for bringing this up.

[mthrok]

Tests look good to me. Thanks! @bhargavkathivarapu

[vincentqb]

LGTM, thanks!