-
Notifications
You must be signed in to change notification settings - Fork 438
make _convert_to_statespace properly pass signal and system names #884
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
make _convert_to_statespace properly pass signal and system names #884
Conversation
…same as transfer functions. tests to track down missing names on some systems when converting to LinearIOSystem
Found it! Losing signal names when slycot is not installed. Working on a fix. |
Found the bug and squashed it. |
@sawyerbfuller Should the system name be copied over directly or should it be modified by appending |
@murrayrm my first thought is that the private method should keep the same system name because it is being used is internally. But maybe if the system was changed by the user using ‘ss’, then that default should be overrided and the system should get a new name. sys$converted? |
I like the idea of having "$converted" appended when you do an explicit conversion (eg, via If we do that, we should probably generate a utility function/method that renames objects, probably in |
HI @murrayrm that sounds good. Yes there are a few issues surrounding when to add For this PR, OK if we leave it as it is, with |
update: this PR has been converted to a bugfix in
_convert_to_statespace
to insure it copies signal and system names correctly. Found the bug by using the test suite.Old PR description:
On some
systemscomputers , the process of convertingtf
systems intoLinearIOSystems
in theinterconnect
function loses signal names -- if the transfer function is static. For example, if a system is defined assys=tf(1, 2, inputs='a', outputs='b')
, an error is raised when that system is interconnected with others becausesys
gets copied but loses its signal names, something along the lines ofSignal 'a' not found
.But this doesn't happen on my system on Python 3.9, 3.10, or 3.11. Maybe something peculiar to windows?
This PR adds unit tests in an attempt to see if our test suite catches this bug.