Split signal.lti class into subclasses
Hi everyone, I started looking into improving the signal.lti class following the issue discussed at https://github.com/scipy/scipy/issues/2912 The pull request can be found here: https://github.com/scipy/scipy/pull/4576 The main idea is to split the lti class into ss, tf, and zpk subclasses. Calling the lti class itself returns instances of these three subclasses. Advantages * No redundant information (lti class currently holds the information of all 3 classes) * Reduce overhead (creating 3 system representations) * Switching between the different subclasses is more explicit: obj.ss(), obj.tf(), obj.zpk() * Avoids one huge class for everything * Is fully backwards compatible (as far as I can tell) * Similar to what Octave / Matlab does (easier to convert code from there to scipy) Disadvantages: * Accessing properties that are not part of the subclass is more expensive (e.g.: sys = ss(1,1,1,1); sys.num --> this now returns sys.tf().num). Any suggestions / comments / things I've broken? Best, Felix
Hi everyone, it would be great if someone could comment on my pull request from a month ago (see below for details) https://github.com/scipy/scipy/pull/4576 This is my first contribution to scipy, so any feedback would be highly appreciated. Thanks, Felix On 01/03/15 16:47, Felix Berkenkamp wrote:
Hi everyone,
I started looking into improving the signal.lti class following the issue discussed at https://github.com/scipy/scipy/issues/2912
The pull request can be found here: https://github.com/scipy/scipy/pull/4576
The main idea is to split the lti class into ss, tf, and zpk subclasses. Calling the lti class itself returns instances of these three subclasses.
Advantages * No redundant information (lti class currently holds the information of all 3 classes) * Reduce overhead (creating 3 system representations) * Switching between the different subclasses is more explicit: obj.ss(), obj.tf(), obj.zpk() * Avoids one huge class for everything * Is fully backwards compatible (as far as I can tell) * Similar to what Octave / Matlab does (easier to convert code from there to scipy)
Disadvantages: * Accessing properties that are not part of the subclass is more expensive (e.g.: sys = ss(1,1,1,1); sys.num --> this now returns sys.tf().num).
Any suggestions / comments / things I've broken?
Best, Felix
Hi, I hope I won't upset anyone saying this, but IMHO the SS/tf stuff in scipy right now is close to unusable so any improvement is a good thing. On Sun, 12 Apr 2015 11:52:58 +0200, Felix Berkenkamp wrote:
Hi everyone,
it would be great if someone could comment on my pull request from a month ago (see below for details) https://github.com/scipy/scipy/pull/4576
This is my first contribution to scipy, so any feedback would be highly appreciated.
Thanks, Felix
On 01/03/15 16:47, Felix Berkenkamp wrote:
Hi everyone,
I started looking into improving the signal.lti class following the issue discussed at https://github.com/scipy/scipy/issues/2912
The pull request can be found here: https://github.com/scipy/scipy/pull/4576
The main idea is to split the lti class into ss, tf, and zpk subclasses. Calling the lti class itself returns instances of these three subclasses.
Advantages * No redundant information (lti class currently holds the information of all 3 classes) * Reduce overhead (creating 3 system representations) * Switching between the different subclasses is more explicit: obj.ss(), obj.tf(), obj.zpk() * Avoids one huge class for everything * Is fully backwards compatible (as far as I can tell) * Similar to what Octave / Matlab does (easier to convert code from there to scipy)
Disadvantages: * Accessing properties that are not part of the subclass is more expensive (e.g.: sys = ss(1,1,1,1); sys.num --> this now returns sys.tf().num).
Any suggestions / comments / things I've broken?
Best, Felix
_______________________________________________ SciPy-Dev mailing list SciPy-Dev@scipy.org http://mail.scipy.org/mailman/listinfo/scipy-dev
On Sun, Apr 12, 2015 at 12:13 PM, Irvin Probst < irvin.probst@ensta-bretagne.fr> wrote:
Hi, I hope I won't upset anyone saying this, but IMHO the SS/tf stuff in scipy right now is close to unusable so any improvement is a good thing.
I don't think anyone will be offended that easily:) It's still not in great shape, but there have been significant improvements over the last few releases with the addition of more functions that use zpk internally and very recently the SOS format.
On Sun, 12 Apr 2015 11:52:58 +0200, Felix Berkenkamp wrote:
Hi everyone,
it would be great if someone could comment on my pull request from a month ago (see below for details) https://github.com/scipy/scipy/pull/4576
This is my first contribution to scipy, so any feedback would be highly appreciated.
Sorry about the delay in reviewing. Your attempt at tackling this is much appreciated - it's one of the more important issues in scipy.signal to address. I'll have a look now.
Thanks, Felix
On 01/03/15 16:47, Felix Berkenkamp wrote:
Hi everyone,
I started looking into improving the signal.lti class following the issue discussed at https://github.com/scipy/scipy/issues/2912
Note that there is more discussion on this topic in several places. Here are two relevant issues: https://github.com/scipy/scipy/issues/2973 (operations on LTI instances) https://github.com/scipy/scipy/issues/3259 (letting filter functions accept zpk/ss input)
The pull request can be found here: https://github.com/scipy/scipy/pull/4576
The main idea is to split the lti class into ss, tf, and zpk subclasses. Calling the lti class itself returns instances of these three subclasses.
After a first browse of your code I still think this is a good idea. We need to think carefully about the API though. First thoughts: - I'd like the classes to have more Pythonic names, instead of too short names that are taken from Matlan - The conversion functions should probably be named "to_zpk, to_ss, "to_tf" rather than "zpk, tf, ss". - There's no SOS class, we do have that format now ( https://github.com/scipy/scipy/pull/3717) - Warren suggested "lti.from_tf/zpk/ss/sos" constructors on issue 3259. Looks like that should fit with your PR, but good to think about now. - I think that this fits with where we want to go with input to filter functions (gh-3259), but ideally we'd work that out together with this PR (not the implementation, but at least a sketch of the new API). Ralf
Advantages * No redundant information (lti class currently holds the information of all 3 classes) * Reduce overhead (creating 3 system representations) * Switching between the different subclasses is more explicit: obj.ss(), obj.tf(), obj.zpk() * Avoids one huge class for everything * Is fully backwards compatible (as far as I can tell) * Similar to what Octave / Matlab does (easier to convert code from there to scipy)
Disadvantages: * Accessing properties that are not part of the subclass is more expensive (e.g.: sys = ss(1,1,1,1); sys.num --> this now returns sys.tf().num).
Any suggestions / comments / things I've broken?
Best, Felix
_______________________________________________ SciPy-Dev mailing list SciPy-Dev@scipy.org http://mail.scipy.org/mailman/listinfo/scipy-dev
_______________________________________________ SciPy-Dev mailing list SciPy-Dev@scipy.org http://mail.scipy.org/mailman/listinfo/scipy-dev
participants (3)
-
Felix Berkenkamp -
Irvin Probst -
Ralf Gommers