JuliaControl / ControlSystems.jl

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

Add constructors with nu, ny, nx for easy rebuilding #399

Closed jonniedie closed 2 years ago

jonniedie commented 3 years ago

Some useful functions that need to recurse through arbitrarily nested structs and rebuild them with different values--like @set and @set! from Setfield.jl or replace_particles (along with its derived functions mean_object and nominal) from MonteCarloMeasurements.jl--require constructors that match the positions of the type's fields. Since TransferFunction and StateSpace have fields for their number of inputs/outputs[/states], but no constructors that take them in, it creates problems for these types of functions. For example, these don't currently work:

julia> using ControlSystems, MonteCarloMeasurements, Setfield

julia> a = zpk([-1±0.1], [im, -im], 5);

julia> @set! a.matrix[1].k = 8
ERROR: MethodError: no method matching TransferFunction(::Array{ControlSystems.SisoZpk{Particles{Float64,2000},Complex{Particles{Float64,2000}}},2}, ::Continuous, ::Int64, ::Int64)
...

julia> mean_object(a)
┌ Error: Failed to create a `TransferFunction` by calling it with its fields in order. For this to work, `TransferFunction` must have a constructor that accepts all fields in the order they appear in the struct and 
accept that the fields that contained particles are replaced by 0. Try defining a meaningful constructor that accepts arguments with the type signature
...

but adding

StateSpace(A, B, C, D, timeevol, nx, nu, ny) = StateSpace(A, B, C, D, timeevol)

makes it so they work:

julia> @set! a.matrix[1].k = 8
TransferFunction{Continuous,ControlSystems.SisoZpk{Particles{Float64,2000},Complex{Particles{Float64,2000}}}}
   1.0s + 1.0 ± 0.1
8.0----------------
     1.0s^2 + 1.0

Continuous-time transfer function model

julia> mean_object(a)
TransferFunction{Continuous,ControlSystems.SisoZpk{Float64,Complex{Float64}}}
    1.0s + 1.0
8.0------------
   1.0s^2 + 1.0

Continuous-time transfer function model

This and the similar method for TransferFunction, are all that's needed for these types of functions to work. I can make a pull request with these, if it would be useful.

baggepinnen commented 3 years ago

I think that these fields were added as a convenience back in the day when one could not overload getproperty. I think it would be safe to add them to getproperty and remove the fields? @mfalt can you see any reason to keep them around, other than the nice way of accessing the sizes?

olof3 commented 3 years ago

I think that these fields were added as a convenience back in the day when one could not overload getproperty. I think it would be safe to add them to getproperty and remove the fields?

I absolutely agree, I have been thinking this a couple of times as well

baggepinnen commented 3 years ago

This should be solved for statespace types now since we removed the internal fields that were no longer required. Might still be a problem for transfer functions though.