-
Notifications
You must be signed in to change notification settings - Fork 32
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
Zonotope order reduction (Girard's algorithm) #211
Conversation
src/Zonotope.jl
Outdated
This function implements the algorithm described in A. Girard's | ||
*Reachability of Uncertain Linear Systems Using Zonotopes*, HSCC. Vol. 5. 2005. | ||
""" | ||
function reduce_order(Z::Zonotope, r::Int) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
output type?
src/Zonotope.jl
Outdated
function reduce_order(Z::Zonotope, r::Int) | ||
c, G = Z.center, Z.generators | ||
p, d = ngens(Z), dim(Z) | ||
h = zeros(p) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zeros(N, p)
(you need to define N
first), assuming that vecnorm
is not always Float64
(I did not check)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked it: For Rational
it returns Float64
, but for Float32
it returns Float32
. So let us keep it as is (Float32
is automatically promoted to Float64
, I checked it).
src/Zonotope.jl
Outdated
end | ||
ind = sortperm(h) | ||
|
||
if r * d >= p |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull up as far as possible? No need to compute h
in that case!
src/Zonotope.jl
Outdated
rg = G[:, ind[1:m]] | ||
|
||
# interval hull computation of reduced generators | ||
d = sum(abs.(rg), 2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this line do? d
is never used again...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the sum is used just below, to build Gbox
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but this line can be removed I think (or use d
below instead).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/Zonotope.jl
Outdated
Gred = Gbox | ||
end | ||
Zred = Zonotope(c, Gred) | ||
return Zred |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return immediately?
src/Zonotope.jl
Outdated
d = sum(abs.(rg), 2) | ||
Gbox = diagm(sum(abs.(rg), 2)[:]) | ||
if m > p | ||
Gred = [G[:, ind[m+1]:end] Gbox] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be ind[m+1:end]
(not sure)? Maybe add a comment what happens here...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ind[m+1]
points to a number (index), so this chooses from that index to the last one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but ind
is a vector of length p
thus the check is nonsense (we need m+1 <= p
), updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
equivalent to m < p
for integers ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, ind[m+1:end]
because these is the set of non reduced generators, well done.
src/Zonotope.jl
Outdated
p, d = ngens(Z), dim(Z) | ||
h = zeros(p) | ||
for i in 1:p | ||
h[i] = vecnorm(G[:, i], 1) - vecnorm(G[:, i], Inf) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For vectors norm
and vecnorm
should be equivalent. Any reason why you used vecnorm
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not particular, no. i tried to use vecnorm
column-wise in the matrix G
with a broadcast but it didn't work, thus the for loop.
Closes #195.