Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert pid parameters to :parallel #900

Merged
merged 8 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Convert pid parameters to :parallel
Using standard fails when KP=0
  • Loading branch information
Michele Zaffalon committed Nov 11, 2023
commit a3b77ebe84a81e9353c86054511f6046c4b13829
91 changes: 60 additions & 31 deletions lib/ControlSystemsBase/src/pid_design.jl
Original file line number Diff line number Diff line change
Expand Up @@ -48,43 +48,41 @@
@deprecate pid(; kp=0, ki=0, kd=0, series = false) pid(kp, ki, kd; form=series ? :series : :parallel)

function pid_tf(param_p, param_i, param_d=zero(typeof(param_p)); form=:standard, Tf=nothing)
Kp, Ti, Td = convert_pidparams_to_standard(param_p, param_i, param_d, form)
ia = Ti != Inf && Ti != 0 # integral action, 0 would result in division by zero, but typically indicates that the user wants no integral action
Kp, Ki, Kd = convert_pidparams_to_parallel(param_p, param_i, param_d, form)

Check warning on line 51 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L51

Added line #L51 was not covered by tests
if isnothing(Tf)
if ia
return tf([Kp * Td, Kp, Kp / Ti], [1, 0])
if Ki != 0
return tf([Kd, Kp, Ki], [1, 0])

Check warning on line 54 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L53-L54

Added lines #L53 - L54 were not covered by tests
else
return tf([Kp * Td, Kp], [1])
return tf([Kd, Kp], [1])

Check warning on line 56 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L56

Added line #L56 was not covered by tests
end
else
if ia
return tf([Kp * Td, Kp, Kp / Ti], [Tf^2/2, Tf, 1, 0])
if Ki != 0
return tf([Kd, Kp, Ki], [Tf^2/2, Tf, 1, 0])

Check warning on line 60 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L59-L60

Added lines #L59 - L60 were not covered by tests
else
return tf([Kp * Td, Kp], [Tf^2/2, Tf, 1])
return tf([Kd, Kp], [Tf^2/2, Tf, 1])

Check warning on line 62 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L62

Added line #L62 was not covered by tests
end
end
end

function pid_ss(param_p, param_i, param_d=zero(typeof(param_p)); form=:standard, Tf=nothing)
Kp, Ti, Td = convert_pidparams_to_standard(param_p, param_i, param_d, form)
Kp, Ki, Kd = convert_pidparams_to_parallel(param_p, param_i, param_d, form)

Check warning on line 68 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L68

Added line #L68 was not covered by tests
TE = Continuous()
ia = Ti != Inf && Ti != 0 # integral action, 0 would result in division by zero, but typically indicates that the user wants no integral action
if !isnothing(Tf)
if ia
if Ki != 0

Check warning on line 71 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L71

Added line #L71 was not covered by tests
A = [0 1 0; 0 0 1; 0 -2/Tf^2 -2/Tf]
B = [0; 0; 1]
C = 2 * Kp / Tf^2 * [1/Ti 1 Td]
C = 2 / Tf^2 * [Ki Kp Kd]

Check warning on line 74 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L74

Added line #L74 was not covered by tests
else
A = [0 1; -2/Tf^2 -2/Tf]
B = [0; 1]
C = 2 * Kp / Tf^2 * [1 Td]
C = 2 / Tf^2 * [Kp Kd]

Check warning on line 78 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L78

Added line #L78 was not covered by tests
end
D = 0
elseif Td == 0
if ia
elseif Kd == 0
if Ki != 0

Check warning on line 82 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L81-L82

Added lines #L81 - L82 were not covered by tests
A = 0
B = 1
C = Kp / Ti # Ti == 0 would result in division by zero, but typically indicates that the user wants no integral action
C = Ki # Ti == 0 would result in division by zero, but typically indicates that the user wants no integral action

Check warning on line 85 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L85

Added line #L85 was not covered by tests
D = Kp
else
return ss([Kp])
Expand All @@ -98,7 +96,7 @@
"""
pidplots(P, args...; params_p, params_i, params_d=0, form=:standard, ω=0, grid=false, kwargs...)

Plots interesting figures related to closing the loop around process `P` with a PID controller supplied in `params`
Display the relevant plots related to closing the loop around process `P` with a PID controller supplied in `params`
on one of the following forms:
* `:standard` - `Kp*(1 + 1/(Ti*s) + Td*s)`
* `:series` - `Kc*(1 + 1/(τi*s))*(τd*s + 1)`
Expand Down Expand Up @@ -491,7 +489,7 @@
verbose && ki < 0 && @warn "Calculated ki is negative, inspect the Nyquist plot generated with doplot = true and try adjusting ω or the angle ϕt"
C = pid(kp, ki, kd, form=:parallel)
any(real(p) > 0 for p in poles(C)) && @error "Calculated controller is unstable."
kp, ki, kd = ControlSystemsBase.convert_pidparams_from_to(kp, ki, kd, :parallel, form)
kp, ki, kd = convert_pidparams_from_to(kp, ki, kd, :parallel, form)

Check warning on line 492 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L492

Added line #L492 was not covered by tests
CF = C*F
fig = if doplot
w = exp10.(LinRange(log10(ω)-2, log10(ω)+2, 1000))
Expand All @@ -511,26 +509,26 @@
end

"""
Kp, Ti, Td = convert_pidparams_to_standard(param_p, param_i, param_d, form)
Kp, Ti, Td = convert_pidparams_to_parallel(param_p, param_i, param_d, form)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to keep the old function as it was and just add the new function. We don't want to unnecessarily break any downstream package that might have use this function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the phone so haven't checked, but iirc we neither have this in online docs or export it, so I probably wouldn't count it as api.
I think the thing that is exported is the convert_pidparams_from_to, though I could be wrong.
But it is also easy to leave it as an extra method if you feel more comfortable with that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to keep it for a few reasons

  • I make use of it in some downstream packages
  • It would be nice to keep the old version around for comparison for a while in case we encounter any strange results with the new method
  • Julia currently (as of v1.9, but soon improved by the new public keyword) lacks a good way to indicate what is and isn't public facing API, and whether or not something is included in the docs is a policy that is awkward to work with from a user perspective. I thus wouldn't be surprised if other people have found this function and made use of it. It does have a nice docstring after all, and we haven't explicitly documented any policy for what is and isn't public API in ControlSystems.


Convert parameters from form `form` to `:standard` form.
Convert parameters from form `form` to `:parallel` form.

The `form` can be chosen as one of the following
* `:standard` - ``K_p(1 + 1/(T_i s) + T_ds)``
* `:series` - ``K_c(1 + 1/(τ_i s))(τ_d s + 1)``
* `:parallel` - ``K_p + K_i/s + K_d s``
"""
function convert_pidparams_to_standard(param_p, param_i, param_d, form::Symbol)
if form === :standard
function convert_pidparams_to_parallel(param_p, param_i, param_d, form::Symbol)
if form === :parallel

Check warning on line 522 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L521-L522

Added lines #L521 - L522 were not covered by tests
return param_p, param_i, param_d
elseif form === :series
return @. (
param_p * (param_i + param_d) / param_i,
param_i + param_d,
param_i * param_d / (param_i + param_d)
)
elseif form === :parallel
return @. (param_p, param_p / param_i, param_d / param_p)
# param_i = 0 would result in division by zero, but typically indicates that the user wants no integral action
param_i == 0 && return @. (param_p * (1, 0, param_d))
return @. (param_p *

Check warning on line 527 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L526-L527

Added lines #L526 - L527 were not covered by tests
((param_i + param_d) / param_i, 1 / param_i, param_d))
elseif form === :standard
param_i == 0 && return @. param_p * (1, 0, param_d)
return @. param_p * (1, 1 / param_i, param_d)

Check warning on line 531 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L529-L531

Added lines #L529 - L531 were not covered by tests
else
throw(ArgumentError("form $(form) not supported."))
end
Expand Down Expand Up @@ -562,10 +560,41 @@
end
end


"""
Kp, Ti, Td = convert_pidparams_from_parallel(param_p, param_i, param_d, to_form)

Convert parameters from form `:parallel` to form `to_form`.

The `form` can be chosen as one of the following
* `:standard` - ``K_p(1 + 1/(T_i s) + T_ds)``
* `:series` - ``K_c(1 + 1/(τ_i s))(τ_d s + 1)``
* `:parallel` - ``K_p + K_i/s + K_d s``
"""
function convert_pidparams_from_parallel(param_p, param_i, param_d, to::Symbol)
if to === :parallel
return param_p, param_i, param_d
elseif to === :series
param_i == 0 && return @. (param_p * (1, 0, param_d))
Δ = param_p^2-4param_i*param_d
Δ < 0 &&

Check warning on line 580 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L574-L580

Added lines #L574 - L580 were not covered by tests
error("The condition KP^2-4KI*KD ≥ 0 is not satisfied: the parameters cannot be converted")
sqrtΔ = sqrt(Δ)
return @. ((param_p - sqrtΔ)/2, (param_p - sqrtΔ)/(2param_i), (param_p + sqrtΔ)/(2param_i))
elseif to === :standard
param_p == 0 && error("Cannot convert to standard form when Kp=0")
param_i == 0 && return @. (param_p, Inf, param_d / param_p)
return @. (param_p, param_p / param_i, param_d / param_p)

Check warning on line 587 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L582-L587

Added lines #L582 - L587 were not covered by tests
else
throw(ArgumentError("form $(form) not supported."))

Check warning on line 589 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L589

Added line #L589 was not covered by tests
end
end


"""
convert_pidparams_from_to(kp, ki, kd, from::Symbol, to::Symbol)
"""
function convert_pidparams_from_to(kp, ki, kd, from::Symbol, to::Symbol)
kp, ki, kd = convert_pidparams_to_standard(kp, ki, kd, from)
convert_pidparams_from_standard(kp, ki, kd, to)
kp, ki, kd = convert_pidparams_to_parallel(kp, ki, kd, from)
convert_pidparams_from_parallel(kp, ki, kd, to)

Check warning on line 599 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L598-L599

Added lines #L598 - L599 were not covered by tests
end
16 changes: 9 additions & 7 deletions lib/ControlSystemsBase/test/test_pid_design.jl
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ C, kp, ki = loopshapingPI(P, ωp, phasemargin=60, form=:parallel, doplot=true)
# tf
@test pid(1.0, 1, 1) == tf(1) + tf(1,[1,0]) + tf([1,0],[1])
@test pid(1.0, Inf, 1) == tf(1) + tf([1, 0], [1])
@test pid(1.0, 0, 1) == tf(1) + tf([1, 0], [1])
@test pid(0.0, 1, 1; form=:parallel) == tf(0) + tf(1,[1,0]) + tf([1,0],[1])
# ss
@test tf(pid(1.0, 1, 0; state_space=true)) == tf(1) + tf(1,[1,0])

Expand Down Expand Up @@ -72,13 +74,13 @@ C, Kp, Ti = placePI(P, 2, 0.7; form=:standard)
@test [Kp, Ti] ≈ [9/5, 9/20]

# Test internal functions convert_pidparams*
params = (2, 3, 0.5)
parallel_params = ControlSystemsBase.convert_pidparams_from_standard(params..., :parallel)
@test parallel_params == (2, 2/3, 1)
@test ControlSystemsBase.convert_pidparams_to_standard(parallel_params..., :parallel) == params
series_params = ControlSystemsBase.convert_pidparams_from_standard(params..., :series)
@test series_params == ((3-sqrt(3))/3, (3-sqrt(3))/2, (3+sqrt(3))/2)
@test ControlSystemsBase.convert_pidparams_to_standard(series_params..., :series) == params
params = (4, 3, 0.5)
mzaffalon marked this conversation as resolved.
Show resolved Hide resolved
standard_params = ControlSystemsBase.convert_pidparams_from_parallel(params..., :standard)
@test standard_params == (4, 4/3, 0.5/4)
@test ControlSystemsBase.convert_pidparams_to_parallel(standard_params..., :standard) == params
series_params = ControlSystemsBase.convert_pidparams_from_parallel(params..., :series)
@test series_params == ((4-sqrt(10))/2, (4-sqrt(10))/6, (4+sqrt(10))/6)
@test all(ControlSystemsBase.convert_pidparams_to_parallel(series_params..., :series) .≈ params)

# lead lag link
a = 1
Expand Down
Loading