Speed up resample with kernel generation modification #2553
Conversation
1. loops are replaced with broadcasting 2. device is cast at end of kernel creation 3. dtype is defaulted to float32 ( float64) does nothing valuable 4. defaults for beta and dtype are moved to function declaration
1. loops are replaced with broadcasting 2. device is cast at end of kernel creation 3. dtype is defaulted to float32 ( float64) does nothing valuable 4. defaults for beta and dtype are moved to function declaration
|
cc @adefossez |
|
|
||
| # we do not use built in torch windows here as we need to evaluate the window |
There was a problem hiding this comment.
can you leave this comment in?
torchaudio/functional/functional.py
Outdated
| beta_tensor = torch.tensor(float(beta)) | ||
| window = torch.i0(beta_tensor * torch.sqrt(1 - (t / lowpass_filter_width) ** 2)) / torch.i0(beta_tensor) | ||
|
|
||
| t *= torch.pi |
There was a problem hiding this comment.
we can leave it using math.pi
torchaudio/functional/functional.py
Outdated
| kernels.append(kernel) | ||
| t = torch.arange(0, -new_freq, -1, dtype=dtype)[:, None, None] / new_freq + idx | ||
| t *= base_freq | ||
| t = t.clamp_(-lowpass_filter_width, lowpass_filter_width) | ||
|
|
||
| scale = base_freq / orig_freq |
There was a problem hiding this comment.
scale can be moved down closer to where it's actually being used, after the window parts
torchaudio/functional/functional.py
Outdated
|
|
||
| kernels = torch.where(t == 0, torch.tensor(1.0).to(t), t.sin() / t) | ||
| kernels *= window * scale | ||
| kernels.to(device) |
There was a problem hiding this comment.
here kernels is being moved to the device after computation, rather than starting out on this device (originally device was set in idx). the GH issue mentions this is preferred since computation is faster on cpu than gpu, but can you additionally run benchmarks where device=gpu to compare this? if you'd like, you could also first remove this change and merge the rest of this PR which looks good, and follow up on this change separately
|
Looking good to me :) |
|
@skim0514 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
| t *= base_freq | ||
| t = t.clamp_(-lowpass_filter_width, lowpass_filter_width) | ||
|
|
||
| # we do not use built in torch windows here as we need to evaluate the window |
There was a problem hiding this comment.
there's a second line to this comment missing
|
@skim0514 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Modification from pull request #2415 to improve resample.
Benchmarked for a 89% time reduction, tested in comparison to original resample method.