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.

Sign up for GitHub

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

Jump to bottom

Switch u layout #480

Merged
merged 15 commits into from
May 12, 2021
Merged

Switch u layout #480

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
e1bc88f
switch u layout for lsim
albheim May 7, 2021
9c23ecf
Update src/timeresp.jl
albheim May 7, 2021
7c80632
More updates, one error in test_timeresp
albheim May 8, 2021
47264cf
Fix tests
albheim May 9, 2021
ad98b33
Change to AbstractVecOrMat
albheim May 9, 2021
4c2566d
Catch CuArray in matrix conversion
albheim May 11, 2021
f4eef18
General zero vectors for x0 to support GPUs
albheim May 11, 2021
dfa7918
Update src/timeresp.jl
albheim May 11, 2021
a77de03
Update src/timeresp.jl
albheim May 11, 2021
ef24346
Move f outside lsim
albheim May 11, 2021
a238dc7
Merge branch 'switch_u_layout' of https://github.com/JuliaControl/Con…
albheim May 11, 2021
9dc5097
Remove GPU compatible x0, save for later
albheim May 11, 2021
ebab7f4
Fix doctest
albheim May 11, 2021
9236684
add kwargs
albheim May 12, 2021
1c4ef6e
Remove variable and generalize type
albheim May 12, 2021
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
10 changes: 5 additions & 5 deletions src/delay_systems.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ function _lsim(sys::DelayLtiSystem{T,S}, Base.@nospecialize(u!), t::AbstractArra
y[:,k] .= sv.saveval[k][2]
end

return y', t, x'
return y, t, x
end

# We have to default to something, look at the sys.P.P and delays
Expand Down Expand Up @@ -244,8 +244,8 @@ function Base.step(sys::DelayLtiSystem{T}, t::AbstractVector; kwargs...) where T
if nu == 1
y, tout, x = lsim(sys, u, t; x0=x0, kwargs...)
else
x = Array{T}(undef, length(t), nstates(sys), nu)
y = Array{T}(undef, length(t), noutputs(sys), nu)
x = Array{T}(undef, nstates(sys), length(t), nu)
y = Array{T}(undef, noutputs(sys), length(t), nu)
for i=1:nu
y[:,:,i], tout, x[:,:,i] = lsim(sys[:,i], u, t; x0=x0, kwargs...)
end
Expand All @@ -264,8 +264,8 @@ function impulse(sys::DelayLtiSystem{T}, t::AbstractVector; alg=MethodOfSteps(BS
if nu == 1
y, tout, x = lsim(sys, u, t; alg=alg, x0=sys.P.B[:,1], kwargs...)
else
x = Array{T}(undef, length(t), nstates(sys), nu)
y = Array{T}(undef, length(t), noutputs(sys), nu)
x = Array{T}(undef, nstates(sys), length(t), nu)
y = Array{T}(undef, noutputs(sys), length(t), nu)
for i=1:nu
y[:,:,i], tout, x[:,:,i] = lsim(sys[:,i], u, t; alg=alg, x0=sys.P.B[:,i], kwargs...)
end
Expand Down
4 changes: 2 additions & 2 deletions src/plotting.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ lsimplot
title --> "System Response"
subplot --> s2i(1,i)
label --> "\$G_{$(si)}\$"
t, y[:, i]
t, y[i, :]
end
end
end
Expand Down Expand Up @@ -208,7 +208,7 @@ for (func, title, typ) = ((step, "Step Response", Stepplot), (impulse, "Impulse
y,t = func(s, p.args[2:end]...)
for i=1:ny
for j=1:nu
ydata = reshape(y[:, i, j], size(t, 1))
ydata = reshape(y[i, :, j], size(t, 1))
style = iscontinuous(s) ? :path : :steppost
ttext = (nu > 1 && i==1) ? title*" from: u($j) " : title
titles[s2i(i,j)] = ttext
Expand Down
4 changes: 2 additions & 2 deletions src/synthesis.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ u(x,t) = -L*x # Form control law,
t=0:0.1:5
x0 = [1,0]
y, t, x, uout = lsim(sys,u,t,x0=x0)
plot(t,x, lab=["Position" "Velocity"], xlabel="Time [s]")
plot(t,x', lab=["Position" "Velocity"], xlabel="Time [s]")
```
"""
function lqr(A, B, Q, R)
Expand Down Expand Up @@ -95,7 +95,7 @@ u(x,t) = -L*x # Form control law,
t=0:Ts:5
x0 = [1,0]
y, t, x, uout = lsim(sys,u,t,x0=x0)
plot(t,x, lab=["Position" "Velocity"], xlabel="Time [s]")
plot(t,x', lab=["Position" "Velocity"], xlabel="Time [s]")
```
"""
function dlqr(A, B, Q, R)
Expand Down
60 changes: 28 additions & 32 deletions src/timeresp.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,20 @@ Calculate the step response of system `sys`. If the final time `tfinal` or time
vector `t` is not provided, one is calculated based on the system pole
locations.

`y` has size `(length(t), ny, nu)`, `x` has size `(length(t), nx, nu)`"""
`y` has size `(ny, length(t), nu)`, `x` has size `(nx, length(t), nu)`"""
function Base.step(sys::AbstractStateSpace, t::AbstractVector; method=:cont)
lt = length(t)
ny, nu = size(sys)
nx = sys.nx
u = (x,t)->[one(eltype(t))]
u = (x,t)->[one(eltype(t))] # 547.109 μs (5240 allocations: 748.75 KiB)
#tmp = [one(eltype(t))] # Slightly less allocations but maybe no real difference, 539.624 μs (4916 allocations: 721.61 KiB)
#u = (x,t)->tmp
x0 = zeros(nx)
if nu == 1
y, tout, x, _ = lsim(sys, u, t, x0=x0, method=method)
else
x = Array{Float64}(undef, lt, nx, nu)
y = Array{Float64}(undef, lt, ny, nu)
x = Array{Float64}(undef, nx, lt, nu)
y = Array{Float64}(undef, ny, lt, nu)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm. Is this how we want it?

I guess this is the best way wrt memory layout, but it does seem quite confusing and inconsistent with the order that would be natural for e.g., freqfresp, where we would have the frequency index last.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought this is what we said for the first two arguments, and the last one made sense to me to leave based on how it was accessed. What would the other alternative be, nx x nu x lt?

Currently freqresp has the frequency index in the first dimension if I'm not mistaken, why would it be the most natural to have it last?

Copy link
Member

Choose a reason for hiding this comment

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

Also, Float64 should probably be made generic?

Copy link
Member

Choose a reason for hiding this comment

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

freqresp probably needs a change corresponding to the one in this PR to place frequency in the last dimension, since a single call to evalfr generates a matrix ny x nu of frequency responses at a single frequency.

Copy link
Member

Choose a reason for hiding this comment

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

One can always use PermutedDimsArray in the end to shape the array the way we want without copying. The user can then, if they choose, call copy on it to obtain a regular array, or use the PermutedDimsArray like it was a regular array

julia> PermutedDimsArray(randn(1,2,3), (2,3,1))
2×3×1 PermutedDimsArray(::Array{Float64, 3}, (2, 3, 1)) with eltype Float64:

Copy link
Contributor

Choose a reason for hiding this comment

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

PermutedDimsArray seems like a reasonable fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would probably be better to avoid introducing lt, consider just using length(t) or possibly call it nt instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, Float64 should probably be made generic?

Seem reasonable, copied the way used in impulse.

Would probably be better to avoid introducing lt, consider just using length(t) or possibly call it nt instead.

I switched to using length(t) instead.

for i=1:nu
y[:,:,i], tout, x[:,:,i],_ = lsim(sys[:,i], u, t, x0=x0, method=method)
end
Expand All @@ -41,7 +43,7 @@ Calculate the impulse response of system `sys`. If the final time `tfinal` or ti
vector `t` is not provided, one is calculated based on the system pole
locations.

`y` has size `(length(t), ny, nu)`, `x` has size `(length(t), nx, nu)`"""
`y` has size `(ny, length(t), nu)`, `x` has size `(nx, length(t), nu)`"""
function impulse(sys::AbstractStateSpace, t::AbstractVector; method=:cont)
T = promote_type(eltype(sys.A), Float64)
lt = length(t)
Expand All @@ -61,8 +63,8 @@ function impulse(sys::AbstractStateSpace, t::AbstractVector; method=:cont)
if nu == 1 # Why two cases # QUESTION: Not type stable?
y, t, x,_ = lsim(sys, u, t, x0=x0s[:], method=method)
else
x = Array{T}(undef, lt, nx, nu)
y = Array{T}(undef, lt, ny, nu)
x = Array{T}(undef, nx, lt, nu)
y = Array{T}(undef, ny, lt, nu)
for i=1:nu
y[:,:,i], t, x[:,:,i],_ = lsim(sys[:,i], u, t, x0=x0s[:,i], method=method)
end
Expand All @@ -81,7 +83,7 @@ impulse(sys::TransferFunction, t::AbstractVector; kwags...) = impulse(ss(sys), t
Calculate the time response of system `sys` to input `u`. If `x0` is ommitted,
a zero vector is used.

`y`, `x`, `uout` has time in the first dimension. Initial state `x0` defaults to zero.
`y`, `x`, `uout` has time in the second dimension. Initial state `x0` defaults to zero.

Continuous time systems are simulated using an ODE solver if `u` is a function. If `u` is an array, the system is discretized before simulation. For a lower level inteface, see `?Simulator` and `?solve`

Expand All @@ -106,7 +108,7 @@ u(x,t) = -L*x # Form control law,
t=0:0.1:5
x0 = [1,0]
y, t, x, uout = lsim(sys,u,t,x0=x0)
plot(t,x, lab=["Position" "Velocity"], xlabel="Time [s]")
plot(t,x', lab=["Position" "Velocity"], xlabel="Time [s]")
```
"""
function lsim(sys::AbstractStateSpace, u::AbstractVecOrMat, t::AbstractVector;
Expand All @@ -117,8 +119,10 @@ function lsim(sys::AbstractStateSpace, u::AbstractVecOrMat, t::AbstractVector;
if length(x0) != nx
error("size(x0) must match the number of states of sys")
end
if !(size(u) in [(length(t), nu) (length(t),)])
error("u must be of size (length(t), nu)")
if size(u) == (length(t),)
reshape!(u, :, 1) # Do we want to modify arguments? Or should this be copied?
else size(u) != (nu, length(t))
error("u must be of size (nu, length(t))")
albheim marked this conversation as resolved.
Show resolved Hide resolved
end

dt = Float64(t[2] - t[1])
Expand All @@ -135,7 +139,7 @@ function lsim(sys::AbstractStateSpace, u::AbstractVecOrMat, t::AbstractVector;
dsys = c2d(sys, dt, :zoh)
elseif method === :foh
dsys, x0map = c2d_x0map(sys, dt, :foh)
x0 = x0map*[x0; transpose(u)[:,1]]
x0 = x0map*[x0; u[:,1]]
else
error("Unsupported discretization method")
end
Expand All @@ -147,7 +151,7 @@ function lsim(sys::AbstractStateSpace, u::AbstractVecOrMat, t::AbstractVector;
end

x = ltitr(dsys.A, dsys.B, u, x0)
y = transpose(sys.C*transpose(x) + sys.D*transpose(u))
y = sys.C*x + sys.D*u
return y, t, x
end

Expand All @@ -165,7 +169,7 @@ function lsim(sys::AbstractStateSpace, u::Function, tfinal::Real, args...; kwarg
end

function lsim(sys::AbstractStateSpace, u::Function, t::AbstractVector;
x0::VecOrMat=zeros(sys.nx), method::Symbol=:cont)
x0::VecOrMat=zeros(sys.nx, 1), method::Symbol=:cont)
ny, nu = size(sys)
nx = sys.nx
u0 = u(x0,1)
Expand All @@ -189,15 +193,12 @@ function lsim(sys::AbstractStateSpace, u::Function, t::AbstractVector;
end
x,uout = ltitr(dsys.A, dsys.B, u, t, T.(x0))
else
s = Simulator(sys, u)
sol = solve(s, T.(x0), (t[1],t[end]), Tsit5())
x = sol(t)'
uout = Array{eltype(x)}(undef, length(t), ninputs(sys))
for (i,ti) in enumerate(t)
uout[i,:] = u(x[i,:],ti)'
end
f(dx,x,p,t) = dx .= sys.A*x .+ sys.B*u(x,t)
Copy link
Member

Choose a reason for hiding this comment

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

Here we have an opportunity to do better in case arrays are static and we take up the direct dependence on StaticArrays. The dx .= will fail for static arrays.

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't used static arrays at all, so not familiar with why that would fail. What would be the static array friendly option?

Copy link
Member

Choose a reason for hiding this comment

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

A static array is static, hence trying to mutate it with dx .= something is not allowed. A static-arrays friendly version would not use any mutation and just write the code as if it allocated, because static arrays are stack-allocated and hence the allocation is free. This code is still correct from a regular array perspective, so any optimizations for static arrays would be in form of another method, i.e.,

f(x::SArray,p,t) = sys.A*x .+ sys.B*u(x,t)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh, thought you only meant that A and B should be static. But everything is going to be static. Sounds interesting, will have to have a look at that.

Should it then be up to the user to define x0 and then the simulation is done with that array type for x?

sol = solve(ODEProblem(f,x0,(t[1],t[end])), Tsit5(); saveat=t)
x = reduce(hcat, sol.u)
uout = reduce(hcat, u(x[:, i], t[i]) for i in eachindex(t))
end
y = transpose(sys.C*transpose(x) + sys.D*transpose(uout))
y = sys.C*x + sys.D*uout
return y, t, x, uout
end

Expand All @@ -223,25 +224,20 @@ If `u` is a function, then `u(x,i)` is called to calculate the control signal ev
T = promote_type(LinearAlgebra.promote_op(LinearAlgebra.matprod, eltype(A), eltype(x0)),
LinearAlgebra.promote_op(LinearAlgebra.matprod, eltype(B), eltype(u)))

n = size(u, 1)

# Transposing u allows column-wise operations, which apparently is faster.
ut = transpose(u)
n = size(u, 2)

# Using similar instead of Matrix{T} to allow for CuArrays to be used.
# This approach is problematic if x0 is sparse for example, but was considered
# to be good enough for now
x = similar(x0, T, (length(x0), n))

x[:,1] .= x0
mul!(x[:, 2:end], B, transpose(u[1:end-1, :])) # Do all multiplications B*u[:,k] to save view allocations
mul!(x[:, 2:end], B, u[:, 1:end-1]) # Do all multiplications B*u[:,k] to save view allocations

tmp = similar(x0, T) # temporary vector for storing A*x[:,k]
for k=1:n-1
mul!(tmp, A, x[:,k])
x[:,k+1] .+= tmp
mul!(x[:, k+1], A, x[:,k], true, true)
end
return transpose(x)
return x
end

function ltitr(A::AbstractMatrix{T}, B::AbstractMatrix{T}, u::Function, t,
Expand All @@ -255,7 +251,7 @@ function ltitr(A::AbstractMatrix{T}, B::AbstractMatrix{T}, u::Function, t,
uout[:,i] .= u(x0,t[i])
x0 = A * x0 + B * uout[:,i]
end
return transpose(x), transpose(uout)
return x, uout
end

# HELPERS:
Expand Down
26 changes: 13 additions & 13 deletions test/test_delayed_systems.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,21 +166,21 @@ println("Simulating first delay system:")
@time step(delay(1)*tf(1,[1,1]))

@time y1, t1, x1 = step([s11;s12], 10)
@time @test y1[:,2] ≈ step(s12, t1)[1] rtol = 1e-14
@time @test y1[2:2,:] ≈ step(s12, t1)[1] rtol = 1e-14

t = 0.0:0.1:10
y2, t2, x2 = step(s1, t)
# TODO Figure out which is inexact here
@test y2[:,1,1:1] + y2[:,1,2:2] ≈ step(s11, t)[1] + step(s12, t)[1] rtol=1e-14
@test y2[1:1,:,1:1] + y2[1:1,:,2:2] ≈ step(s11, t)[1] + step(s12, t)[1] rtol=1e-14

y3, t3, x3 = step([s11; s12], t)
@test y3[:,1,1] ≈ step(s11, t)[1] rtol = 1e-14
@test y3[:,2,1] ≈ step(s12, t)[1] rtol = 1e-14
@test y3[1:1,:,1] ≈ step(s11, t)[1] rtol = 1e-14
@test y3[2:2,:,1] ≈ step(s12, t)[1] rtol = 1e-14

y1, t1, x1 = step(DelayLtiSystem([1.0/s 2/s; 3/s 4/s]), t)
y2, t2, x2 = step([1.0/s 2/s; 3/s 4/s], t)
@test y1 ≈ y2 rtol=1e-14
@test size(x1,1) == length(t)
@test size(x1,2) == length(t)
@test size(x1,3) == 2


Expand All @@ -202,7 +202,7 @@ function y_expected(t, K)
end
end

@test ystep ≈ y_expected.(t, K) atol = 1e-12
@test ystep' ≈ y_expected.(t, K) atol = 1e-12

function dy_expected(t, K)
if t < 1
Expand All @@ -219,13 +219,13 @@ end
y_impulse, t, _ = impulse(sys_known, 3)

# TODO Better accuracy for impulse
@test y_impulse ≈ dy_expected.(t, K) rtol=1e-13
@test maximum(abs, y_impulse - dy_expected.(t, K)) < 1e-12
@test y_impulse' ≈ dy_expected.(t, K) rtol=1e-13
@test maximum(abs, y_impulse' - dy_expected.(t, K)) < 1e-12

y_impulse, t, _ = impulse(sys_known, 3, alg=MethodOfSteps(Tsit5()))
# Two orders of magnitude better with BS3 in this case, which is default for impulse
@test y_impulse ≈ dy_expected.(t, K) rtol=1e-5
@test maximum(abs, y_impulse - dy_expected.(t, K)) < 1e-5
@test y_impulse' ≈ dy_expected.(t, K) rtol=1e-5
@test maximum(abs, y_impulse' - dy_expected.(t, K)) < 1e-5

## Test delay with D22 term
t = 0:0.01:4
Expand All @@ -234,16 +234,16 @@ sys = delay(1)

y, t, x = step(sys, t)
@test y[:] ≈ [zeros(100); ones(301)] atol = 1e-12
@test size(x) == (401,0)
@test size(x) == (0,401)

sys = delay(1)*delay(1)*1/s

y, t, x = step(sys, t)

y_sol = [zeros(200);0:0.01:2]
y_sol = [zeros(200);0:0.01:2]'

@test maximum(abs,y-y_sol) < 1e-13
@test maximum(abs,x-collect(0:0.01:4)) < 1e-15
@test maximum(abs,x-collect(0:0.01:4)') < 1e-15

# TODO For some reason really bad accuracy here
# Looks like a lag in time
Expand Down
Loading