JuliaControl / ControlSystems.jl

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

c2d does not maintain type #441

Closed albheim closed 2 years ago

albheim commented 3 years ago

Noticed that when using c2d on a zpk system it changes type in the returned system. Seems like it could be nice to have the type stay the same.

julia> G = zpk([], [-1.0], 1)
TransferFunction{Continuous, ControlSystems.SisoZpk{Float64, Float64}}
      1.0
1.0----------
   1.0s + 1.0

Continuous-time transfer function model

julia> c2d(G, 0.1)
TransferFunction{Discrete{Float64}, ControlSystems.SisoRational{Float64}}
   0.09516258196404037
-------------------------
1.0z - 0.9048374180359594

Sample Time: 0.1 (seconds)
Discrete-time transfer function model
olof3 commented 3 years ago

Absolutely, this is something that should be fixed.

I guess the easy way is just to add a conversion, but I guess we really should use pole--zero mapping. Seems to be pretty straightforward to implement. I guess there are other conversion methods that I'm not aware of, but I think this is a very common one.

albheim commented 3 years ago

Gave a conversion a quick try and got a bit confused at how to handle types correctly. Have not tried the pole zero mapping yet but seems like it should be rather easy. But should not this be an option for the method keyword in c2d rather than being the automatic option? Or maybe it could be the default for a zpk system, but also be available as a keyword for other system representations?

albheim commented 3 years ago

Looked at the pole zero matching and implemented it for zpk types, and it seems to work. The thing I am a bit unsure about is how to nicely handle the dispatch when this is added. Before everything would be discretized through state space and there is the c2d_x0map that contains all the different transformations, and transferfunctions are just transformed and sent here. If we now have pz matching it seems like this would mess up this structure a bit and might need some more work than I was up for right now (i.e. we dont want pz matching to be in c2d_x0map since then we will transform zpk->ss->zpk in the process.