Proposal: calling signal.lti object returns transfer function value
Hello all, I've been trying the scipy.signal module lately for some easy things and I had an idea, but I thought it would be better to bring it up in the mailing list before filing a PR on GitHub. My proposal is to implement the __call__ method on signal.lti objects so it returns the value on the transfer function. I took the idea from the `bode` function. This line: y = numpy.polyval(sys.num, jw) / numpy.polyval(sys.den, jw) (https://github.com/scipy/scipy/blob/v0.12.0/scipy/signal/ltisys.py#L940) could be moved to signal.lti.__call__, and then in `bode` write y = sys(jw) The detail is trivial (so is the implementation) but I think it would be a useful thing. What do others think? Shall I open a pull request?
BTW (and this is a side question), is there a reason the code of signal.lti is full of `self.__dict__['X'] = Y` instead of `self.X = Y`? https://github.com/scipy/scipy/blob/v0.12.0/scipy/signal/ltisys.py#L262
On Tue, Jul 16, 2013 at 5:28 PM, Juan Luis Cano <juanlu001@gmail.com> wrote:
BTW (and this is a side question), is there a reason the code of signal.lti is full of `self.__dict__['X'] = Y` instead of `self.X = Y`?
It's very ugly but does have a function. Because of how ``__setattr__`` is implemented you cannot just set self.num & related attributes in ``__init__``. The alternative is to transform num/den/zeros/poles/.... into properties with setter methods that keep all properties in sync. A PR making that change would be welcome. Ralf
On 07/20/2013 05:54 PM, Ralf Gommers wrote:
On Tue, Jul 16, 2013 at 5:28 PM, Juan Luis Cano <juanlu001@gmail.com <mailto:juanlu001@gmail.com>> wrote:
BTW (and this is a side question), is there a reason the code of signal.lti is full of `self.__dict__['X'] = Y` instead of `self.X = Y`?
It's very ugly but does have a function. Because of how ``__setattr__`` is implemented you cannot just set self.num & related attributes in ``__init__``. The alternative is to transform num/den/zeros/poles/.... into properties with setter methods that keep all properties in sync. A PR making that change would be welcome.
If I understood correctly, it would be something like this: @property num(self): """Numerator""" return self._num @num.setter num(self, value): self._num = value self.__dict__['zeros'], self.__dict__['poles'], self.dict__['gain'] = tf2zpk(self.num, self.den) self.__dict__['A'], self.__dict__['B'], self.dict__['C'], self.dict__['D'] = tf2ss(self.num, self.den) then you really cannot get rid of `self.__dict__['X']` because if you use the `self.X` then the same circular problem would happen with the setters of the other properties. And on the other hand, when instantiating the object not all the properties are available (e.g.: you assign `num`, but cannot update zeros, poles and gain because `den` is not yet assigned) so a special case has to be included in the setter. What about, instead of storing redundant information on the object, a "canonical" representation is used, for instance (A, B, C, D), and getters are created that compute (num, den) and (zeros, poles, gain)? Maybe one could even cache these properties so they would be computed only once anyway.
On Tue, Jul 23, 2013 at 6:05 PM, Juan Luis Cano <juanlu001@gmail.com> wrote:
On 07/20/2013 05:54 PM, Ralf Gommers wrote:
On Tue, Jul 16, 2013 at 5:28 PM, Juan Luis Cano <juanlu001@gmail.com>wrote:
BTW (and this is a side question), is there a reason the code of signal.lti is full of `self.__dict__['X'] = Y` instead of `self.X = Y`?
It's very ugly but does have a function. Because of how ``__setattr__`` is implemented you cannot just set self.num & related attributes in ``__init__``. The alternative is to transform num/den/zeros/poles/.... into properties with setter methods that keep all properties in sync. A PR making that change would be welcome.
If I understood correctly, it would be something like this:
@property num(self): """Numerator""" return self._num
@num.setter num(self, value): self._num = value self.__dict__['zeros'], self.__dict__['poles'], self.dict__['gain'] = tf2zpk(self.num, self.den) self.__dict__['A'], self.__dict__['B'], self.dict__['C'], self.dict__['D'] = tf2ss(self.num, self.den)
then you really cannot get rid of `self.__dict__['X']` because if you use the `self.X` then the same circular problem would happen with the setters of the other properties.
The setter methods can use the private attributes to get around that, for example num.setter updates self._den and not self.den. And on the other hand, when instantiating the object not all the properties
are available (e.g.: you assign `num`, but cannot update zeros, poles and gain because `den` is not yet assigned) so a special case has to be included in the setter.
Not really, for the same reason. Assign to self._num and self._den.
What about, instead of storing redundant information on the object, a "canonical" representation is used, for instance (A, B, C, D), and getters are created that compute (num, den) and (zeros, poles, gain)? Maybe one could even cache these properties so they would be computed only once anyway.
Should also work, but will likely be more complicated. Ralf
On 07/23/2013 11:47 PM, Ralf Gommers wrote:
On Tue, Jul 23, 2013 at 6:05 PM, Juan Luis Cano <juanlu001@gmail.com <mailto:juanlu001@gmail.com>> wrote:
On 07/20/2013 05:54 PM, Ralf Gommers wrote:
On Tue, Jul 16, 2013 at 5:28 PM, Juan Luis Cano <juanlu001@gmail.com <mailto:juanlu001@gmail.com>> wrote:
BTW (and this is a side question), is there a reason the code of signal.lti is full of `self.__dict__['X'] = Y` instead of `self.X = Y`?
It's very ugly but does have a function. Because of how ``__setattr__`` is implemented you cannot just set self.num & related attributes in ``__init__``. The alternative is to transform num/den/zeros/poles/.... into properties with setter methods that keep all properties in sync. A PR making that change would be welcome.
If I understood correctly, it would be something like this:
@property num(self): """Numerator""" return self._num
@num.setter num(self, value): self._num = value self.__dict__['zeros'], self.__dict__['poles'], self.dict__['gain'] = tf2zpk(self.num, self.den) self.__dict__['A'], self.__dict__['B'], self.dict__['C'], self.dict__['D'] = tf2ss(self.num, self.den)
then you really cannot get rid of `self.__dict__['X']` because if you use the `self.X` then the same circular problem would happen with the setters of the other properties.
The setter methods can use the private attributes to get around that, for example num.setter updates self._den and not self.den.
And on the other hand, when instantiating the object not all the properties are available (e.g.: you assign `num`, but cannot update zeros, poles and gain because `den` is not yet assigned) so a special case has to be included in the setter.
Not really, for the same reason. Assign to self._num and self._den.
Thank you very much for your advice! I implemented these ideas in this pull request: https://github.com/scipy/scipy/pull/2655 I hope we can follow the discussion there and improve the code. Regards Juan Luis Cano
On Tue, Jul 16, 2013 at 4:49 PM, Juan Luis Cano <juanlu001@gmail.com> wrote:
Hello all,
I've been trying the scipy.signal module lately for some easy things and I had an idea, but I thought it would be better to bring it up in the mailing list before filing a PR on GitHub.
My proposal is to implement the __call__ method on signal.lti objects so it returns the value on the transfer function.
I took the idea from the `bode` function. This line:
y = numpy.polyval(sys.num, jw) / numpy.polyval(sys.den, jw)
(https://github.com/scipy/scipy/blob/v0.12.0/scipy/signal/ltisys.py#L940)
could be moved to signal.lti.__call__, and then in `bode` write
y = sys(jw)
The detail is trivial (so is the implementation) but I think it would be a useful thing. What do others think? Shall I open a pull request?
Not sure, is writing y = lti_sys(jw) instead of jw, y = lti_sys.freqresp(jw) clearer and umambiguous? It may make sense, but would be good to get the opinion of some users on this. Ralf
On 07/20/2013 05:39 PM, Ralf Gommers wrote:
On Tue, Jul 16, 2013 at 4:49 PM, Juan Luis Cano <juanlu001@gmail.com <mailto:juanlu001@gmail.com>> wrote:
Hello all,
I've been trying the scipy.signal module lately for some easy things and I had an idea, but I thought it would be better to bring it up in the mailing list before filing a PR on GitHub.
My proposal is to implement the __call__ method on signal.lti objects so it returns the value on the transfer function.
I took the idea from the `bode` function. This line:
y = numpy.polyval(sys.num, jw) / numpy.polyval(sys.den, jw)
(https://github.com/scipy/scipy/blob/v0.12.0/scipy/signal/ltisys.py#L940)
could be moved to signal.lti.__call__, and then in `bode` write
y = sys(jw)
The detail is trivial (so is the implementation) but I think it would be a useful thing. What do others think? Shall I open a pull request?
Not sure, is writing y = lti_sys(jw) instead of jw, y = lti_sys.freqresp(jw) clearer and umambiguous? It may make sense, but would be good to get the opinion of some users on this.
Ralf
I am sorry but I was unaware of the freqresp function, probably it's the clearest way. Maybe it should be used in the `bode` function too. Juan Luis Cano
On 7/23/13, Juan Luis Cano <juanlu001@gmail.com> wrote:
On 07/20/2013 05:39 PM, Ralf Gommers wrote:
On Tue, Jul 16, 2013 at 4:49 PM, Juan Luis Cano <juanlu001@gmail.com <mailto:juanlu001@gmail.com>> wrote:
Hello all,
I've been trying the scipy.signal module lately for some easy things and I had an idea, but I thought it would be better to bring it up in the mailing list before filing a PR on GitHub.
My proposal is to implement the __call__ method on signal.lti objects so it returns the value on the transfer function.
I took the idea from the `bode` function. This line:
y = numpy.polyval(sys.num, jw) / numpy.polyval(sys.den, jw)
(https://github.com/scipy/scipy/blob/v0.12.0/scipy/signal/ltisys.py#L940)
could be moved to signal.lti.__call__, and then in `bode` write
y = sys(jw)
The detail is trivial (so is the implementation) but I think it would be a useful thing. What do others think? Shall I open a pull request?
Not sure, is writing y = lti_sys(jw) instead of jw, y = lti_sys.freqresp(jw) clearer and umambiguous? It may make sense, but would be good to get the opinion of some users on this.
Ralf
I am sorry but I was unaware of the freqresp function, probably it's the clearest way. Maybe it should be used in the `bode` function too.
There was some work on bode and freqresp earlier this year. The latest version of `bode` uses `freqresp`: https://github.com/scipy/scipy/blob/master/scipy/signal/ltisys.py#L856 Warren
Juan Luis Cano
On 07/23/2013 02:09 PM, Warren Weckesser wrote:
There was some work on bode and freqresp earlier this year. The latest version of `bode` uses `freqresp`: https://github.com/scipy/scipy/blob/master/scipy/signal/ltisys.py#L856
Thank you for pointing it out, I was reading the source from my locally installed last tagged version.
participants (3)
-
Juan Luis Cano -
Ralf Gommers -
Warren Weckesser