JuliaControl / ControlSystems.jl

A Control Systems Toolbox for Julia
https://juliacontrol.github.io/ControlSystems.jl/stable/
Other
513 stars 85 forks source link

ss not working when Ts is assigned #196

Closed hurak closed 5 years ago

hurak commented 5 years ago

It appears that the ss constructor is not working when Ts keyword argument is assigned. In particular, when a discrete-time state-space model is created with a given period.

using ControlSystems

a = b = c = 1.0
d = 0.0
G = ss(a,b,c,d,Ts=1.0)

ERROR: LoadError: MethodError: no method matching ss(::Float64, ::Float64, ::Float64, ::Float64; Ts=1.0) Closest candidates are: ss(::Union{Number, AbstractArray}, ::Union{Number, AbstractArray}, ::Union{Number, AbstractArray}, ::Union{Number, AbstractArray}) at /home/hurak/.julia/packages/ControlSystems/6Nac2/src/types/ss.jl:23 got unsupported keyword argument "Ts" ss(::Union{Number, AbstractArray}, ::Union{Number, AbstractArray}, ::Union{Number, AbstractArray}, ::Union{Number, AbstractArray}, ::Real) at /home/hurak/.julia/packages/ControlSystems/6Nac2/src/types/ss.jl:23 got unsupported keyword argument "Ts" ss(::Number, ::Real; kwargs...) at /home/hurak/.julia/packages/ControlSystems/6Nac2/src/types/ss.jl:44 ... Stacktrace: [1] top-level scope at none:0 in expression starting at /home/hurak/ownCloud/julia/lsim_breaking_for_discrete.jl:5

mfalt commented 5 years ago

The ss constuctor does not take Ts as a keyword argument, the documentation says sys = ss(A, B, C, D, Ts=0) -> sys, note the difference to ss(A, B, C, D; Ts=0). I will close this since it is not a bug. Feel free to reopen if you think the interface should be difference, or if anything should be clarified.

hurak commented 5 years ago

The ss constuctor does not take Ts as a keyword argument, the documentation says sys = ss(A, B, C, D, Ts=0) -> sys, note the difference to ss(A, B, C, D; Ts=0). I will close this since it is not a bug. Feel free to reopen if you think the interface should be difference, or if anything should be clarified.

Being a newbie to programming in Julia, I confess that I still have troubles to understand sys = ss(A, B, C, D, Ts=0) -> sys. I do understand the role of the -> symbol in defining functions but I do not understand its role here. Anyway, although I am willing to learn, my guess is that other new users will expect the same usage of the constructor as I (mistakenly) did: ss(A, B, C, D, Ts=0).

In my opinion the Ts attribute is not any special from other attributes and parameters - perhaps later we might even want to add the names of the input/state/output variables or perhaps even the units or whatever other attributes/parameters to the ss "object" - and a more traditional way of calling the constructors might be preferable.

mfalt commented 5 years ago

You are right that the "-> sys" doesn't really makes sense, and we should remove it, it seems to have stuck when we changed the syntax in the documentation from @doc """ss(A,B,C,D[, Ts, statenames=..., inputnames=..., outputnames=...]) -> sys`

Note that we did have state, input and output names. But they were heavily bloating the code, ambigous under any sort of operations (adding, multiplying, etc), and added very litte benefit, except for in plots (where you can now set your own labels thanks to plot recipes), so we decided to remove them.

mfalt commented 5 years ago

Regarding the traditional constructors. This is a large work related to what we have been working on for quite a while on improving, mostly related to how the type system works. See for example the wiki pages https://github.com/JuliaControl/ControlSystems.jl/wiki/Restructuring-ControlSystems and https://github.com/JuliaControl/ControlSystems.jl/wiki/Type-system-suggestion-for-the-toolbox-2

No final decisions have been made here, so we are still open for suggestions, although we do have some idead of where we want to take it.

We have started defining the more "julian" constructors, see for example

StateSpace{T,MT}(A::AbstractArray, B::AbstractArray, C::AbstractArray, D::AbstractArray, Ts::Real) where {T, MT <: AbstractMatrix{T}}

at https://github.com/JuliaControl/ControlSystems.jl/blob/master/src/types/StateSpace.jl#L41

However, since we are still working on updating the types and type parameters, it is probably not wise to push them on the users yet. The long term goal is probably that "ss/tf/zpk" could be a convinient way of defining types, maybe with some extra defaults for new users or as compatibility with what users are expecting from matlab. For example ensuring that systems with complex or integer coeffiencts aren't created by accident.

It is great that you are looking at the package and using it. The more people using it, the more bugs and unexpected behaviours we find. Please keep reporting the issues when you find them.