facets(::Cone) docstring

[fingolfin]

The docstring for facets(::Cone) says this:

    facets(as::Type{T} = LinearHalfspace, C::Cone)

Return the facets of `C` in the format defined by `as`.

The allowed values for `as` are
* `Halfspace`,
* `Cone`.

Several issues:

1. the signature suggests that the default value for as is LinearHalfspace, but the text instead talks about Halfspace. Experimentally, only the latter works, but perhaps I am doing it wrong?

julia> c = positive_hull([1 0 0; 0 1 0; 1 1 1])
Polyhedral cone in ambient dimension 3

julia> facets(Halfspace, c)
3-element SubObjectIterator{LinearHalfspace{QQFieldElem}} over the Halfspaces of R^3 described by:
-x₃ ≦ 0
-x₁ + x₃ ≦ 0
-x₂ + x₃ ≦ 0


julia> facets(LinearHalfspace, c)
ERROR: MethodError: no method matching facets(::Type{LinearHalfspace}, ::Cone{QQFieldElem})

Looking at the code I guess it might work if I specified LinearHalfspace{T} for some value of T?

2. What I really wanted to do was to use as = Cone, but that didn't work either (similar problem, I think: I need Cone{T}, but from the docs this is not clear, plus it is not clear which value for {T}, plus this is inconvenient):

julia> facets(Cone, c)
ERROR: MethodError: no method matching facets(::Type{Cone}, ::Cone{QQFieldElem})

---

Possible solution: there is this method:

facets(::Type{Halfspace}, C::Cone{T}) where T<:scalar_types = facets(LinearHalfspace{T}, C)

perhaps we should add more siblings to it? I.e.:

facets(::Type{AffineHalfspace}, C::Cone{T}) where T<:scalar_types = facets(AffineHalfspace{T}, C)
facets(::Type{LinearHalfspace}, C::Cone{T}) where T<:scalar_types = facets(LinearHalfspace{T}, C)
facets(::Type{Polyhedron}, C::Cone{T}) where T<:scalar_types = facets(Polyhedron{T}, C)
facets(::Type{Cone}, C::Cone{T}) where T<:scalar_types = facets(Cone{T}, C)

and similar for Polyhedron instead of Cone?

Does that sound reasonable?

Should the additional as values be listed in the respective docstrings?

[lkastner]

Your suggestions make sense.

However in light of what we talked about in #2126 before I proceed I'd like to know what you expect mathematically as output for the variants facets(Cone, C) or facets(Polyhedron, P)? I know what I would want, but I want to impose any bias at this point. Also @antonydellavecchia and @benlorenz

[fingolfin]

Late reply to that question: I'd expect that for fs = facets(T, C) the return value is a vector such that fs[i] isa T for all $i$. So if facets(Cone, C) returns a Vector{Cone{X}} for whatever X, I'd be happy. And indeed that seems to have been implemented and works nicely:

julia> c = positive_hull([1 0 0; 0 1 0; 1 1 1])
Polyhedral cone in ambient dimension 3

julia> facets(Cone, c)
3-element SubObjectIterator{Cone{QQFieldElem}}:
 Polyhedral cone in ambient dimension 3
 Polyhedral cone in ambient dimension 3
 Polyhedral cone in ambient dimension 3

and also

julia> C = cube(3);

julia> facets(Polyhedron, C)
6-element SubObjectIterator{Polyhedron{QQFieldElem}}:
 Polytope in ambient dimension 3
 Polytope in ambient dimension 3
 Polytope in ambient dimension 3
 Polytope in ambient dimension 3
 Polytope in ambient dimension 3
 Polytope in ambient dimension 3

julia> facets(Cone, c)
3-element SubObjectIterator{Cone{QQFieldElem}}:
 Polyhedral cone in ambient dimension 3
 Polyhedral cone in ambient dimension 3
 Polyhedral cone in ambient dimension 3

Also facets(LinearHalfspace, c) now works.

So it seems to me the only thing left to be done here is to improve the docstring?

Because it still says facets(as::Type{T} = LinearHalfspace, C::Cone) but then the list of allowed values for as does not include LinearHalfspace. Similar for the Polyhedron method.

And there are two docstrings fro facets(as::Type{T} = AffineHalfspace, P::Polyhedron) and facets(P::Polyhedron) but the later is already covered by the former via that default value, so perhaps they can be merged?