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