JuliaReach / LazySets.jl

Scalable symbolic-numeric set computations in Julia
https://juliareach.github.io/LazySets.jl/
Other
226 stars 32 forks source link

Use constraints_list when converting from polytope to HPolytope #932

Closed schillic closed 5 years ago

schillic commented 5 years ago

Currently we have this implementation:

function convert(::Type{HPolytope}, P::AbstractPolytope)
    return convert(HPolytope, convert(VPolytope, P))
end

It would be better to not use the V-representation. Every AbstractPolytope provides the constraints_list method now.

There is one problem with Zonotope, however, where the constraints_list method falls back to convert. So we first need #416.

mforets commented 5 years ago

Since #416 is now done, shall we reconsider this issue? The fallback implementation to convert from an abstract polytope to an HPolytope is still the one quoted above (see this function).

I think that

function convert(::Type{HPolytope}, P::AbstractPolytope)
    return convert(HPolytope, convert(VPolytope, P))
end

function convert(::Type{HPolyhedron}, P::AbstractPolytope)
    return HPolyhedron(constraints_list(P))
end

can be unified into

function convert(T::Union{Type{HPolytope}, Type{HPolyhedron}}, P::AbstractPolytope;
                 use_constraint_representation=true)
    if use_constraint_representation
        return T(constraints_list(P))
    else
        return convert(T, convert(VPolytope, P))
    end
end
schillic commented 5 years ago

No need for the flag. To create these two types, you always need a list of constraints. So this implementation only makes sense for types that do not support constraints_list, but they cannot be AbstractPolytopes then.

mforets commented 5 years ago

True, thanks for the correction. Either the inner call convert(VPolytope, P) or the outer one convert(T, ...) needs to call a constraints_list under the hood.