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

Fix firwin() calls with estimate_filter_length() #100

Closed
MattCarrickPL opened this issue May 8, 2023 · 2 comments · Fixed by #116
Closed

Fix firwin() calls with estimate_filter_length() #100

MattCarrickPL opened this issue May 8, 2023 · 2 comments · Fixed by #116
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@MattCarrickPL
Copy link
Collaborator

Cataloging firwin() calls that need:

  1. a derived transition bandwidth, rather than hard coded (ex: 0.02)
  2. Number of taps from estimate_filter_length()
  3. a specified fs=1 (firwin assumes fs=2)

from transforms/signal_processing/functional.py:

70 num_taps = int(2np.ceil(502np.pi/new_rate/.125/22)) # fred harris rule of thumb * 2
71 taps = signal.firwin(
72 num_taps,
73 new_rate
0.98,
74 width=new_rate * .02,
75 window=signal.get_window("blackman", num_taps),
76 scale=True
77 )

from transforms/signal_processing/sp_functional.py:

70 num_taps = int(2np.ceil(502np.pi/new_rate/.125/22)) # fred harris rule of thumb * 2
71 taps = signal.firwin(
72 num_taps,
73 new_rate
0.98,
74 width=new_rate * .02,
75 window=signal.get_window("blackman", num_taps),
76 scale=True
77 )

from transforms/system_impairment/si_functional.py:

115 num_taps = int(2np.ceil(502*np.pi/(1/up)/.125/22)) # fred harris rule of thumb * 2
116 taps = sp.firwin(
117 num_taps,
118 (1/up),
119 width=(1/up) * .02,
120 window=sp.get_window("blackman", num_taps),
121 scale=True
122 )

130 num_taps = int(2 * np.ceil(50 * 2 * np.pi / (1/up) / .125 / 22)) # fred harris rule-of-thumb * 2
131 taps = sp.firwin(
132 num_taps,
133 1 / up,
134 width=(1/up) * .02,
135 window=sp.get_window("blackman", num_taps),
136 scale=True
137 )

354 taps = sp.firwin(
355 num_taps,
356 bandwidth,
357 width=bandwidth * .02,
358 window=sp.get_window("blackman", num_taps),
359 scale=True
360 )

Similar function calls in transforms/system_impairment/functional.py

@gvanhoy gvanhoy added this to the v0.3.1 milestone May 12, 2023
@gvanhoy gvanhoy added the enhancement New feature or request label May 12, 2023
@gvanhoy
Copy link
Collaborator

gvanhoy commented May 12, 2023

We don't need a whole filter design suite, but at least a function that gives a reasonably-well designed filter for what we tend to use it for in this library would be great.

@MattCarrickPL
Copy link
Collaborator Author

We can still use firwin() but we need to update the function calls. It uses a filter length approximation that generates filters that are too long and assumes a static transition bandwidth of 1% which is quite narrow and impractical for real world systems.

@gvanhoy gvanhoy modified the milestones: v0.3.1, v0.3.2, v0.4.0 May 22, 2023
@gvanhoy gvanhoy self-assigned this May 25, 2023
@gvanhoy gvanhoy linked a pull request May 25, 2023 that will close this issue
@gvanhoy gvanhoy closed this as completed May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants