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: freqz does not work on multi-dimensional array, despite what documentation claims #17387

Open
sliedes opened this issue Nov 10, 2022 · 3 comments · May be fixed by #17699 or #20308
Open

BUG: freqz does not work on multi-dimensional array, despite what documentation claims #17387

sliedes opened this issue Nov 10, 2022 · 3 comments · May be fixed by #17699 or #20308
Labels
Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org good first issue Good topic for first contributor pull requests, with a relatively straightforward solution scipy.signal

Comments

@sliedes
Copy link

sliedes commented Nov 10, 2022

Describe your issue.

freqz documentation seems to imply that it supports multidimensional inputs as long as the first dimension is the filter coeffients:

b: array_like

Numerator of a linear filter. If b has dimension greater than 1, it is assumed that the coefficients are stored in the first dimension, and b.shape[1:], a.shape[1:], and the shape of the frequencies array must be compatible for broadcasting.

However, freqz fails when given a 2-dimensional array.

Reproducing Code Example

import scipy
import numpy as np

scipy.signal.freqz(np.zeros((128, 10)))

Error message

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/sami/.virtualenvs/tmp/lib/python3.10/site-packages/scipy/signal/_filter_design.py", line 474, in freqz
    h = (npp_polyval(zm1, b, tensor=False) /
  File "/home/sami/.virtualenvs/tmp/lib/python3.10/site-packages/numpy/polynomial/polynomial.py", line 754, in polyval
    c0 = c[-1] + x*0
ValueError: operands could not be broadcast together with shapes (10,) (512,)

SciPy/NumPy/Python version information

1.9.3 1.23.4 sys.version_info(major=3, minor=10, micro=6, releaselevel='final', serial=0)

@sliedes sliedes added the defect A clear bug or issue that prevents SciPy from being installed or used as expected label Nov 10, 2022
@tupui tupui added the good first issue Good topic for first contributor pull requests, with a relatively straightforward solution label Nov 10, 2022
@tupui
Copy link
Member

tupui commented Nov 10, 2022

Hi @sliedes, thank you for reporting. The documentation could probably be improved, if you have a look at the broadcasting example at the end of the docstrings, you need to be careful with the shape (provided this makes any sense, the following would work scipy.signal.freqz(np.zeros((128, 10, 1)))). Does that help? https://scipy.github.io/devdocs/reference/generated/scipy.signal.freqz.html

We could maybe improve the error messaging here.

@tupui tupui added Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org and removed defect A clear bug or issue that prevents SciPy from being installed or used as expected labels Nov 10, 2022
@tinaoberoi
Copy link

Hi @tupui ,
Is anybody working on this? If not would love to take this up.

@tupui
Copy link
Member

tupui commented Dec 6, 2022

Hi @tinaoberoi, you are more than welcome to work on this. We don't do issue/work assignment, we just make sure there is just 1 PR open for a given task so there is no competing work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org good first issue Good topic for first contributor pull requests, with a relatively straightforward solution scipy.signal
Projects
None yet
4 participants