Polyhedron class mistreats empty inputs

[darijgr]

sage: Polyhedron(ambient_dim=0, ieqs=[], eqns=[], base_ring=QQ)
The empty polyhedron in QQ^0

This should be a one-point polyhedron.

sage: Polyhedron(ambient_dim=2, ieqs=[], eqns=[], base_ring=QQ)
The empty polyhedron in QQ^2

This should be the whole QQ^2.

sage: Polyhedron(ambient_dim=2, vertices=[], rays=[], lines=[], base_ring=QQ)
The empty polyhedron in QQ^2

This is correct, but only because the code misunderstands the empty V-representation as no V-representation given at all.

CC: @vbraun @mkoeppe @tscrim

Component: geometry

Author: Darij Grinberg, Jonathan Kliem

Branch/Commit: c5bf041

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/17339

[vbraun]

comment:1

For the case with rays but no vertex we follow cdd in implicitly adding the origin as vertex. This is also spelled out in the docs.

[darijgr]

comment:2

There is a difference between vertices=[] and vertices=None. In the latter case, we should follow the cdd convention. In the former, we should not; this kind of exception conflicts with regular usage and forces writers of methods to special-case empty vertex sets. Currently the constructor loses the difference between [] and None at one of its first steps (the _make_listlist call); this is what I think needs to be fixed.

[vbraun]

comment:3

I don't really like to make a distinction between None and [], Python generally treats them as similar. Both are not __nonzero__. Its a bad UI to assign meaning to one over the other.

[darijgr]

comment:4

What we have right now is way worse. Say I write some code which constructs a ton of polytopes from their Vreps (e.g., I have a polygon and I construct all subpolygons from some of its vertices). A few of them have empty vertices lists. Why should they be understood differently from the rest?

[vbraun]

comment:5

The design choice is: nothing specified = empty polyhedron.

This is fine with your example, empty vertex list = empty polygon. Unless you mean a non-compact 2-d polyhedron.

I agree that there is perhaps too much DWIM but its also very convenient for interactive use. If you just specify a bunch of rays then in 99% of the cases you want a cone, not some rule thumping about missing a vertex.

How about separate Polyhedron.from_Vrep(vertices, rays, lines) and similarly from_Hrep with positional arguments and strict rules?

[darijgr]

comment:6

Very good idea about the from_Vrep and from_Hrep methods. Should the usual __init__ then dispatch to them?

[vbraun]

comment:7

Replying to @darijgr:

Should the usual __init__ then dispatch to them?

Haven't really thought about it, whatever is convenient for the implementation.

[mkoeppe]

comment:9

Even if it's not good style to distinguish between None and [], I think it is fine to distinguish between a provided keyword argument and a not-provided keyword argument.

So perhaps we should just change the default value of eqns from None to some unique value such as "not_given".

Then users who want to explicitly pass an empty list of eqns could either pass eqns=None or eqns=[].

[tscrim]

comment:11

I think Python quite often distinguishes between [] and None:

sage: list([])
[]
sage: list(None)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-1-f23beda2b661> in <module>()
----> 1 list(None)

TypeError: 'NoneType' object is not iterable

sage: for x in []:
....:     print("hi")
sage: for x in None:
....:     print("hi")
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-5-b89ce2509dd6> in <module>()
----> 1 for x in None:
      2     print("hi")

TypeError: 'NoneType' object is not iterable

They are completely different objects in Python. Other languages behave in the same way for their list-like objects and equivalent of NULL.

I agree that it can be useful to have [] and None behave the same way, but it is not good practice to say we should not distinguish between the two. None is also faster, easier, and safer to work with than some other (essentially equally) generic input.

[kliem]

Author: ​Darij Grinberg, Jonathan Kliem

[kliem]

Changed commit from f7642d6 to f3c0bfb

[kliem]

Changed branch from public/polytopes/0 to public/17339

[kliem]

comment:12

I took the half-backed fix and did some changes that seem to make it work.

It seems that fixing this ticket implies a fixing #28654. The verification of the double description for backend field relied on mistreatment of empty input (or fixing #28654).

---

New commits:

f3c0bfb

fix empty input for Polyhedron class

[sagetrac-git]

Changed commit from f3c0bfb to c5bf041

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

c5bf041

added a doctest, some simplification

[kliem]

comment:15

I propose leaving the following as it is:

sage: Polyhedron(ambient_dim=2, vertices=[], rays=[(2,2)], lines=[], base_ring=QQ)
A 1-dimensional polyhedron in QQ^2 defined as the convex hull of 1 vertex and 1 ray

[tscrim]

comment:16

Should we consider that as valid input? Given a single ray, I don't think there is enough information to determine a polyhedron. However, since that is the current behavior, I cannot strictly object to it. I am only with accepting the ticket as-is since it does improve things, but perhaps we can take a quick second look at the current corner case behaviors?

[kliem]

comment:17

I think if vertices is None then it makes sense to add a vertex.

However, if vertices is [] and there are Rays or Lines, maybe it makes sense to throw an error. The user specifically stated that there are no vertices, so the input doesn't make any sense.

[tscrim]

comment:18

But you cannot infer what the vertices should be from a single ray (or even line). This should be true more generally for any non-full-dimensional polyhedron that cannot specify at least one point inside it. However, as I write that, it seems like a (computationally) hard problem to me to verify this information. Although maybe this is already implicit in how the polyhedron is constructed?

[kliem]

comment:19

I guess it is just convention, to add a vertex, when rays and lines are given, but no vertex.
I don't really care about that convention one way or the other. I just left it the way it is.

I don't understand what part is computationally hard.

[tscrim]

comment:20

I was thinking given enough rays, but now I realize that no matter how much affine data you have, you need to specify a vertex. However, what about a mixture of lines and rays. Although I guess it is documented that the origin is added if no vertices are specified, so it is enshrined as the behavior. Thus this point is moot.

[tscrim]

Reviewer: Travis Scrimshaw

[vbraun]

Changed author from ​Darij Grinberg, Jonathan Kliem to Darij Grinberg, Jonathan Kliem

[vbraun]

comment:21

fixed zero width space in the author name

[vbraun]

Changed branch from public/17339 to c5bf041