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 2 commits
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
102 changes: 66 additions & 36 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 All @@ -550,22 +548,54 @@
if form === :standard
return Kp, Ti, Td
elseif form === :series
return @. (
(Ti - sqrt(Ti * (Ti - 4 * Td))) / 2 * Kp / Ti,
(Ti - sqrt(Ti * (Ti - 4 * Td))) / 2,
(Ti + sqrt(Ti * (Ti - 4 * Td))) / 2,
)
Δ = Ti * (Ti - 4 * Td)
Copy link
Member

Choose a reason for hiding this comment

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

These lines are broken for parameter arrays

Δ < 0 && throw(DomainError("The condition Ti^2 ≥ 4Td*Ti is not satisfied: the PID parameters cannot be converted to series form"))
sqrtΔ = sqrt(Δ)
return @. ((Ti - sqrtΔ) / 2 * Kp / Ti,

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

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L551-L554

Added lines #L551 - L554 were not covered by tests
(Ti - sqrtΔ) / 2,
(Ti + sqrtΔ) / 2)
elseif form === :parallel
return @. (Kp, Kp/Ti, Td*Kp)
else
throw(ArgumentError("form $(form) not supported."))
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 581 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L575-L581

Added lines #L575 - L581 were not covered by tests
throw(DomainError("The condition KP^2 ≥ 4KI*KD is not satisfied: the PID parameters cannot be converted to series form"))
sqrtΔ = sqrt(Δ)
return @. ((param_p - sqrtΔ)/2, (param_p - sqrtΔ)/(2param_i), (param_p + sqrtΔ)/(2param_i))
elseif to === :standard
param_p == 0 && throw(DomainError("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 588 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L583-L588

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

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

View check run for this annotation

Codecov / codecov/patch

lib/ControlSystemsBase/src/pid_design.jl#L590

Added line #L590 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 600 in lib/ControlSystemsBase/src/pid_design.jl

View check run for this annotation

Codecov / codecov/patch

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

Added lines #L599 - L600 were not covered by tests
end
21 changes: 14 additions & 7 deletions lib/ControlSystemsBase/test/test_pid_design.jl
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
@testset "test_pid_design" begin

CSB = ControlSystemsBase

# Test gof plot and loopshaping
P = tf(1,[1,1])^4
gangoffourplot(P,tf(1))
Expand All @@ -13,6 +15,11 @@ 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])
@test_throws DomainError CSB.convert_pidparams_from_parallel(2, 3, 0.5, :series)
@test_throws DomainError CSB.convert_pidparams_from_parallel(0, 3, 0.5, :standard)
@test_throws DomainError CSB.convert_pidparams_from_standard(2, 1, 0.5, :series)
# ss
@test tf(pid(1.0, 1, 0; state_space=true)) == tf(1) + tf(1,[1,0])

Expand Down Expand Up @@ -72,13 +79,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