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

RootRaisedCosine impulse response seem incorrect #59

Open
dan-1d opened this issue Apr 15, 2020 · 5 comments
Open

RootRaisedCosine impulse response seem incorrect #59

dan-1d opened this issue Apr 15, 2020 · 5 comments

Comments

@dan-1d
Copy link

dan-1d commented Apr 15, 2020

When comparing a RRC impulse response generated by CommPy to those of other reference implementations, the results seem incorrect -- e.g., not even symmetric.
I don't have screenshots at the moment, since I changed implementations, but may try to show evidence here.

Just a warning to those using it for pulse shaping.. it's not perfect.

@thomasfillon
Copy link

Indeed, all the filters functions return non-symmetric filters.

The time indexes are defined as:

T_delta = 1/float(Fs)
time_idx = ((np.arange(N)-N/2))*T_delta

As an example we have:

  • for N = 5, T=_delta = 1, time_idx = [-2.5, -1.5, -0.5, 0.5, 1.5]
  • for N = 6, T=_delta = 1, time_idx = [-3., -2., -1., 0., 1., 2.]

Shouldn't the time indexes be symmetric like the following ?:

  • for N=5, time_idx = [-2., -1, 0. 1., 2.]
  • for N=6, time_idx = [-2.5, -1.5, -0.5, 0.5, 1.5 , 2.5]

which correspond to:
time_idx = (np.arange(N)-(N-1)/2) * T_delta

@BastienTr
Copy link
Collaborator

Hi @thomasfillon,

This could make sense. As I do not know enough about the module to provide a conclusive opinion, could you make the change and compare the result with a reference? That way, we would be assured that the issue is properly resolved.

@thomasfillon
Copy link

thomasfillon commented Sep 30, 2020

Hi @BastienTr ,

OK, I will send a pull request with the proposed code modification and some tests.
@dan-1d What reference implementations were you comparing with ?

@dan-1d
Copy link
Author

dan-1d commented Sep 30, 2020

@thomasfillon Good question! I think it was a MATLAB implementation in the "comm" toolbox, but it could have been in-house code. We ended up writing our own in Python, and I haven't gotten back to this since... I can perhaps compare the fix at some point.

@mobilinkd
Copy link

If you happen to choose a an even number of taps (80 in this case), no error is reported, but this is what you get:

pos, taps = rrcosfilter(int(samples_per_symbol * 8), 0.5, 1.0/symbol_rate, sample_rate)

bad_rrc

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

4 participants