JuliaControl / ControlSystems.jl

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

Converting "identity" transfert function with `ss` results in `BoundsError` #828

Closed franckgaga closed 1 year ago

franckgaga commented 1 year ago

Hi,

With the last version (both stable or dev version), calling using ControlSystemsBase and:

mytf = [tf(1,[1,1]) tf(0); tf(0) tf(1,[1,1])]
ss(mytf)

results in this error:

ERROR: BoundsError: attempt to access 0-element Vector{Float64} at index [1]
Stacktrace:
  [1] getindex
    @ ./array.jl:924 [inlined]
  [2] siso_tf_to_ss(T::Type, f::ControlSystemsBase.SisoRational{Int64})
    @ ControlSystemsBase ~/.julia/dev/ControlSystems/lib/ControlSystemsBase/src/types/conversion.jl:140
  [3] #146
    @ ./none:0 [inlined]
  [4] iterate
    @ ./generator.jl:47 [inlined]
  [5] collect_to!
    @ ./array.jl:845 [inlined]
  [6] collect_to_with_first!
    @ ./array.jl:823 [inlined]
  [7] collect(itr::Base.Generator{Vector{ControlSystemsBase.SisoRational{Int64}}, ControlSystemsBase.var"#146#148"{Float64}})
    @ Base ./array.jl:797
  [8] convert(::Type{StateSpace{Continuous, Float64}}, G::TransferFunction{Continuous, ControlSystemsBase.SisoRational{Int64}}; balance::Bool)
    @ ControlSystemsBase ~/.julia/dev/ControlSystems/lib/ControlSystemsBase/src/types/conversion.jl:98
  [9] convert
    @ ~/.julia/dev/ControlSystems/lib/ControlSystemsBase/src/types/conversion.jl:90 [inlined]
 [10] #convert#144
    @ ~/.julia/dev/ControlSystems/lib/ControlSystemsBase/src/types/conversion.jl:86 [inlined]
 [11] convert
    @ ~/.julia/dev/ControlSystems/lib/ControlSystemsBase/src/types/conversion.jl:84 [inlined]
 [12] #StateSpace#33
    @ ~/.julia/dev/ControlSystems/lib/ControlSystemsBase/src/types/StateSpace.jl:127 [inlined]
 [13] StateSpace
    @ ~/.julia/dev/ControlSystems/lib/ControlSystemsBase/src/types/StateSpace.jl:127 [inlined]
 [14] #ss#34
    @ ~/.julia/dev/ControlSystems/lib/ControlSystemsBase/src/types/StateSpace.jl:144 [inlined]
 [15] ss(args::TransferFunction{Continuous, ControlSystemsBase.SisoRational{Int64}})
    @ ControlSystemsBase ~/.julia/dev/ControlSystems/lib/ControlSystemsBase/src/types/StateSpace.jl:144
 [16] top-level scope
    @ REPL[29]:1

It was working as intended in earlier version.

Thanks for all your work !

baggepinnen commented 1 year ago

Hello,

This appears to work correctly here

julia> using ControlSystemsBase
[ Info: Precompiling ControlSystemsBase [aaaaaaaa-a6ca-5380-bf3e-84a91bcd477e]

julia> mytf = [tf(1,[1,1]) tf(0); tf(0) tf(1,[1,1])]
TransferFunction{Continuous, ControlSystemsBase.SisoRational{Int64}}
Input 1 to output 1
  1
-----
s + 1

Input 1 to output 2
0
-
1

Input 2 to output 1
0
-
1

Input 2 to output 2
  1
-----
s + 1

Continuous-time transfer function model

julia> ss(mytf)
StateSpace{Continuous, Float64}
A = 
 -1.0   0.0
  0.0  -1.0
B = 
 1.0  0.0
 0.0  1.0
C = 
 1.0  0.0
 0.0  1.0
D = 
 0.0  0.0
 0.0  0.0

Continuous-time state-space model

are you sure you have not somehow overloaded any of the functions involved to changed their behavior?

Which version of Julia are you using?

PS. The word you're looking for is a diagonal operator ;) DS.

baggepinnen commented 1 year ago

Actually, I can reproduce the error on julia v1.9, let me have a look

baggepinnen commented 1 year ago

The problem was introduced by

The PR above works around the problem, but I'm not convinced the change to Polynomials is correct so I opened an issue there as well