polyhedron redundant constraints

[mforets]

See #1038.

• check if fixes Empty polyhedra intersection is broken #843

[mforets]

intersection could return an instance of ∅ whenever the intersection is empty; we already have this union return type for other set intersections.

[mforets]

perhaps this should instead throw an assertion error, to be checked from the outside (e.g. from intersection)

[schillic]

What is "the polyhedron"? And why should an empty intersection not be allowed?
I think this case should just return an EmptySet.

[mforets]

i threw an error because remove_redundant_constraints! is supposed to modify an HPolyhedron in place. changing P to an emptyset breaks type stability.

otherwise we could change the signature to something like:

function remove_redundant_constraints!(P::Union{PT<:HPoly{N}, EmptySet{N}},
                                       backend=GLPKSolverLP()) where {N}
....

but this could break type stability in other places, no? like functions that use remove_redundant_constraints! should be aware that it can return empty sets.

so just throwing an error seems ok to me.

[schillic]

Oh, I thought this was in intersection. But why would you have to throw an error? Can we not just print a warning and return the original set?

[mforets]

what type of error should be thrown here?

[schillic]

Why does it matter?

[mforets]

thinking of a beter practice, because it helps to debug for example:

              remove_redundant_constraints!(P)
       catch y
           if isa(y, DomainError)
                  return EmptySet()
           elseif isa(y, BoundsError)
                   . . . .
           else  
                   ...
            end
       end

[schillic]

Then you have to study all cases when this can happen. I would say that this is not worth the effort.

[schillic]

I think we need to clarify the cases when we want to use _leq etc. Do we want to overapproximate or underapproximate the result? I think without an answer there is no point in using approximations.
The documentation should clearly mention the behavior for floating point, too.
In this case, I would say that we should not remove more constraints but rather less - to be on the safe side. So here I would check for the opposite condition and use !_geq(...) (but this does not work for exact numbers because _geq is not exactly the opposite of _leq).

[mforets]

ok. for this change, i was having in mind #754 (but i haven't tried it). then, i'll remove this commit which can be done in a separate PR.

i think that once the floating point tolerance has been specified, like a ztol, it doesn't matter if the result is under or overapproximated, one just wants that objval is either strictly smaller than b[j] or above but close to it. this is the purpose of _leq that is a relaxed version of <= for floating point. for rationals this is exact, and for FP we can pass the tolerances through an argument to remove_redundant_constraints!.

[schillic]

i was having in mind #754

You relaxed the condition for removing, so you removed more constraints.

[mforets]

ok. the examples there use Polyhedra.removehredundancy!, i should try in the first place how does it behave with remove_redundant_constriants!

[schillic]

Apart from the _leq thing I am happy here.