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.

Sign up for GitHub

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

Jump to bottom

BUG: fix signal.dlsim for complex input #13075

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

BUG: fix signal.dlsim for complex input #13075

wants to merge 3 commits into from

Conversation

IguanaAzul
Copy link

Reference issue

The problem was that when trying to simulate a system with complex values in the state space matrices, it would perform a matrix multiplication between the input and output matrices, and the output matrices were float by default, so the multiplication casted everyone of them to float, removing the imaginary parts of the complex numbers, so the simulation was wrong.

What does this implement/fix?

I just set the type of the output matrices to be the same as the matrix A in the input, so that it works if the input is complex.

Additional information

@IguanaAzul IguanaAzul mentioned this pull request Nov 12, 2020
Closed
@rgommers rgommers changed the title https://docs.scipy.org/doc/numpy/dev/development_workflow.html#fixed-dlsim-for-complex-inputs BUG: fix signal.dlsim for complex input Nov 14, 2020
@rgommers
Copy link
Member

Thanks @IguanaAzul! Can you please add a test case to signal/tests/test_dltisys.py that fails before this fix and works after it?

It looks like this fixes gh-4964, which already has an example that can be turned into a test case. More comprehensive testing is always welcome, but one case should be enough.

@rgommers rgommers added maintenance Items related to regular maintenance tasks scipy.signal labels Nov 15, 2020
@IguanaAzul
Copy link
Author

Thanks @IguanaAzul! Can you please add a test case to signal/tests/test_dltisys.py that fails before this fix and works after it?

It looks like this fixes gh-4964, which already has an example that can be turned into a test case. More comprehensive testing is always welcome, but one case should be enough.

Sure, I added a test case for that

@rgommers
Copy link
Member

The test you added is failing on almost all CI jobs @IguanaAzul, and also locally for me. Can you have another look?

@IguanaAzul
Copy link
Author

The test you added is failing on almost all CI jobs @IguanaAzul, and also locally for me. Can you have another look?

Oh, sorry, I'll check it.

@j-bowhay j-bowhay added the needs-work Items that are pending response from the author label Apr 4, 2023
tikuma-lsuhsc added a commit to tikuma-lsuhsc/scipy that referenced this pull request Jul 15, 2023
@tikuma-lsuhsc
BUG: signal.dlsim: properly setting output dtype (scipy#13075)
dc35913 
This commit addresses the bug reported in (stale) PR scipy#13075 with a wider
scope:

1. Fixes complex-valued operation. If any SS parameters, input signal,
or initial state is complex valued, the output and final state are also
complex valued.

2. Better FP precision control. The output and final state are set to
the highest precision of the input and system parameters.
@tikuma-lsuhsc tikuma-lsuhsc mentioned this pull request Jul 15, 2023
Open
tikuma-lsuhsc added a commit to tikuma-lsuhsc/scipy that referenced this pull request Jul 16, 2023
@tikuma-lsuhsc
BUG: signal.dlsim: properly setting output dtype (scipy#13075)
8573e5a 
This commit addresses the bug reported in (stale) PR scipy#13075 with a wider
scope:

1. Fixes complex-valued operation. If any SS parameters, input signal,
or initial state is complex valued, the output and final state are also
complex valued.

2. Better FP precision control. The output and final state are set to
the highest precision of the input and system parameters.
tikuma-lsuhsc added a commit to tikuma-lsuhsc/scipy that referenced this pull request Jul 16, 2023
@tikuma-lsuhsc
BUG: signal.dlsim: properly setting output dtype (scipy#13075)
9fda2b1 
This commit addresses the bug reported in (stale) PR scipy#13075 with a wider
scope:

1. Fixes complex-valued operation. If any SS parameters, input signal,
or initial state is complex valued, the output and final state are also
complex valued.

2. Better FP precision control. The output and final state are set to
the highest precision of the input and system parameters.
@roryyorke roryyorke mentioned this pull request Dec 28, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
maintenance Items related to regular maintenance tasks needs-work Items that are pending response from the author scipy.signal
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@IguanaAzul @rgommers @j-bowhay