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

Bug when calculating pre-emphasis filter for plotting #16

Open
ivanpondal opened this issue Dec 6, 2020 · 4 comments
Open

Bug when calculating pre-emphasis filter for plotting #16

ivanpondal opened this issue Dec 6, 2020 · 4 comments

Comments

@ivanpondal
Copy link

Hello!

When plotting, the following function is not calculating correctly the pre-emphasis filter:

return np.concatenate([x, np.subtract(x, np.multiply(x, coeff))])

The correct implementation is being used in the model:

return torch.cat((x[:, :, 0:1], x[:, :, 1:] - coeff * x[:, :, :-1]), dim=2)

At high level this is given a signal x, in order to calculate x[t] you do:
x[t] = x[t] - 0.95*x[t - 1]

btw, great project! 👍

@ivanpondal ivanpondal changed the title Bug when calculating pre-emphasis filter Bug when calculating pre-emphasis filter for plotting Dec 6, 2020
@mishushakov mishushakov mentioned this issue Dec 8, 2020
8 tasks
@GuitarML
Copy link
Owner

GuitarML commented Dec 8, 2020

@ivanpondal Thanks for catching that, I will check it out, and I'll also check that in the GuitarLSTM repo because I might be making the same mistake there.

@GuitarML
Copy link
Owner

GuitarML commented Feb 3, 2021

@ivanpondal Sorry for the delay in responding to this, but if you compare the two implementations, aren't they doing the same thing? The one in plot code is using numpy arrays and the one in the model is using pytorch tensors.

@ivanpondal
Copy link
Author

Hey! The problem is it's not using the previous element for the substraction. The pytorch implementation does:
x[:, :, 1:] - coeff * x[:, :, :-1]
That list slicing produces the desired x[t] = x[t] - 0.95*x[t - 1] while the numpy implementation does the substraction over the same list element.

@GuitarML
Copy link
Owner

GuitarML commented Feb 3, 2021

Got it now, good catch!

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

No branches or pull requests

2 participants