Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: update ApplyBackgroundNoise augmentation #48

Merged
merged 20 commits into from
Dec 1, 2020

Conversation

hbredin
Copy link
Collaborator

@hbredin hbredin commented Nov 23, 2020

No description provided.

@hbredin
Copy link
Collaborator Author

hbredin commented Nov 23, 2020

This is a work in progress but I'd love to receive early feedback anyway.

@iver56 iver56 self-requested a review November 23, 2020 10:00
Copy link
Collaborator

@iver56 iver56 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add torchaudio>=0.6.0 to the list of required dependencies in setup.py

@iver56
Copy link
Collaborator

iver56 commented Nov 23, 2020

I can take another look when it's out of draft :)

hbredin and others added 2 commits November 30, 2020 16:14
Co-authored-by: Iver Jordal <1470603+iver56@users.noreply.github.com>
Co-authored-by: Iver Jordal <1470603+iver56@users.noreply.github.com>
@iver56
Copy link
Collaborator

iver56 commented Nov 30, 2020

@hbredin hbredin changed the title wip: update ApplyBackgroundNoise augmentation feat: update ApplyBackgroundNoise augmentation Nov 30, 2020
@hbredin hbredin marked this pull request as ready for review November 30, 2020 15:34
@hbredin hbredin requested a review from iver56 November 30, 2020 15:35
@hbredin
Copy link
Collaborator Author

hbredin commented Nov 30, 2020

This PR now fails because tests try to augment 2d samples.
Shouldn't we first come up with a PR that switches to 3d only (as discussed last week in slack)?

@iver56
Copy link
Collaborator

iver56 commented Nov 30, 2020

This PR now fails because tests try to augment 2d samples.
Shouldn't we first come up with a PR that switches to 3d only (as discussed last week in slack)?

Yes, we should adapt (or remove) tests that currently provide 2d input. We can enforce/assert 3d input in a different pull request.

@iver56
Copy link
Collaborator

iver56 commented Nov 30, 2020

I tried to use this transform in the demo script. I proposed a few changes in your branch here: hbredin#1

I'm currently getting an exception like this:

Traceback (most recent call last):
  File "C:/Users/Iver/Code/torch-audiomentations/scripts/demo.py", line 139, in <module>
    samples=samples, sample_rate=SAMPLE_RATE
  File "C:\Users\Iver\Anaconda3\envs\torch-audiomentations-gpu\lib\site-packages\torch\nn\modules\module.py", line 722, in _call_impl
    result = self.forward(*input, **kwargs)
  File "C:\Users\Iver\Code\torch-audiomentations\torch_audiomentations\core\transforms_interface.py", line 189, in forward
    self.randomize_parameters(cloned_samples, sample_rate)
  File "C:\Users\Iver\Code\torch-audiomentations\torch_audiomentations\augmentations\background_noise.py", line 113, in randomize_parameters
    [self.random_background(audio, num_samples) for _ in range(batch_size)]
  File "C:\Users\Iver\Code\torch-audiomentations\torch_audiomentations\augmentations\background_noise.py", line 113, in <listcomp>
    [self.random_background(audio, num_samples) for _ in range(batch_size)]
  File "C:\Users\Iver\Code\torch-audiomentations\torch_audiomentations\augmentations\background_noise.py", line 80, in random_background
    0, background_num_samples - missing_num_samples
  File "C:\Users\Iver\Anaconda3\envs\torch-audiomentations-gpu\lib\random.py", line 222, in randint
    return self.randrange(a, b+1)
  File "C:\Users\Iver\Anaconda3\envs\torch-audiomentations-gpu\lib\random.py", line 195, in randrange
    raise ValueError("non-integer stop for randrange()")
ValueError: non-integer stop for randrange()

Edit: I think it's because get_num_samples sometimes doesn't return an int

@iver56
Copy link
Collaborator

iver56 commented Dec 1, 2020

I tried to run the demo script, and I got an exception like this:

Traceback (most recent call last):
  File "C:/Users/Iver/Code/torch-audiomentations/scripts/demo.py", line 139, in <module>
    samples=samples, sample_rate=SAMPLE_RATE
  File "C:\Users\Iver\Anaconda3\envs\torch-audiomentations-gpu\lib\site-packages\torch\nn\modules\module.py", line 722, in _call_impl
    result = self.forward(*input, **kwargs)
  File "C:\Users\Iver\Code\torch-audiomentations\torch_audiomentations\core\transforms_interface.py", line 189, in forward
    self.randomize_parameters(cloned_samples, sample_rate)
  File "C:\Users\Iver\Code\torch-audiomentations\torch_audiomentations\augmentations\background_noise.py", line 113, in randomize_parameters
    [self.random_background(audio, num_samples) for _ in range(batch_size)]
  File "C:\Users\Iver\Code\torch-audiomentations\torch_audiomentations\augmentations\background_noise.py", line 113, in <listcomp>
    [self.random_background(audio, num_samples) for _ in range(batch_size)]
  File "C:\Users\Iver\Code\torch-audiomentations\torch_audiomentations\augmentations\background_noise.py", line 84, in random_background
    background_path, sample_offset=sample_offset, num_samples=num_samples,
  File "C:\Users\Iver\Code\torch-audiomentations\torch_audiomentations\utils\io.py", line 240, in __call__
    raise ValueError()
ValueError

Somehow the numbers don't add up in this case, and I think it's related to signal and noise having different original sample rates

        # io.py line 239-240
        if original_sample_offset + original_num_samples > original_total_num_samples:
            raise ValueError()

Could you try to run the demo script and see if you can reproduce it?

python -m scripts.demo

@iver56
Copy link
Collaborator

iver56 commented Dec 1, 2020

LGTM! 🚀
Thanks for the contribution 😄

@iver56 iver56 merged commit 062347f into asteroid-team:master Dec 1, 2020
@iver56 iver56 removed their request for review December 1, 2020 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants