sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.36k stars 462 forks source link

Need to convert values before passing them to `cdd` #30328

Closed kliem closed 4 years ago

kliem commented 4 years ago
sage: P = polytopes.regular_polygon(5, exact=True)
sage: P1 = polytopes.regular_polygon(5, exact=False)
sage: P*P1
...
ValueError: *Input Error: Input format is not correct.
*Format:
 begin
   m   n  NumberType(real, rational or integer)
   b  -A
 end

We fix this, by converting the values to reals, before passing them to cdd.

We enable a doctest prepared by #30293, which implicitly checks that this is fixed.

The above failure will be implicitly added by #29934.

Depends on #30293

CC: @jplab @LaisRast

Component: geometry

Author: Jonathan Kliem

Branch/Commit: 0be9ed3

Reviewer: Matthias Koeppe

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

kliem commented 4 years ago
comment:2

This was detected in #29934.

kliem commented 4 years ago
comment:3

Ok, maybe its not so much about AA:

sage: tfcell = polytopes.twenty_four_cell(backend='normaliz') 
sage: tfcell.lawrence_extension(tfcell.vertices()[0].vector()*1.0)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-14-3a950b1911b2> in <module>()
----> 1 tfcell.lawrence_extension(tfcell.vertices()[Integer(0)].vector()*RealNumber('1.0'))

/home/jonathan/Applications/sage/local/lib/python3.7/site-packages/sage/geometry/polyhedron/base.py in lawrence_extension(self, v)
   5965         lambda_V = [u + [0] for u in V if u != v] + [v+[1]] + [v+[2]]
   5966         parent = self.parent().base_extend(vector(v), ambient_dim=self.ambient_dim() + 1)
-> 5967         return parent.element_class(parent, [lambda_V, [], []], None)
   5968 
   5969     def lawrence_polytope(self):

/home/jonathan/Applications/sage/local/lib/python3.7/site-packages/sage/geometry/polyhedron/backend_cdd.py in __init__(self, parent, Vrep, Hrep, **kwds)
    518             sage: TestSuite(p).run()
    519         """
--> 520         Polyhedron_cdd.__init__(self, parent, Vrep, Hrep, **kwds)
    521 
    522     def _init_from_Vrepresentation_and_Hrepresentation(self, Vrep, Hrep, verbose=False):

/home/jonathan/Applications/sage/local/lib/python3.7/site-packages/sage/geometry/polyhedron/base.py in __init__(self, parent, Vrep, Hrep, Vrep_minimal, Hrep_minimal, pref_rep, **kwds)
    247 
    248             if vertices or rays or lines:
--> 249                 self._init_from_Vrepresentation(vertices, rays, lines, **kwds)
    250             else:
    251                 self._init_empty_polyhedron()

/home/jonathan/Applications/sage/local/lib/python3.7/site-packages/sage/geometry/polyhedron/backend_cdd.py in _init_from_Vrepresentation(self, vertices, rays, lines, verbose)
     60         from .cdd_file_format import cdd_Vrepresentation
     61         s = cdd_Vrepresentation(self._cdd_type, vertices, rays, lines)
---> 62         s = self._run_cdd(s, '--redcheck', verbose=verbose)
     63         s = self._run_cdd(s, '--repall', verbose=verbose)
     64         self._init_from_cdd_output(s)

/home/jonathan/Applications/sage/local/lib/python3.7/site-packages/sage/geometry/polyhedron/backend_cdd.py in _run_cdd(self, cdd_input_string, cmdline_arg, verbose)
    168         if 'Error:' in ans + err:
    169             # cdd reports errors on stdout and misc information on stderr
--> 170             raise ValueError(ans.strip())
    171         return ans
    172 

ValueError: *Input Error: Input format is not correct.
*Format:
 begin
   m   n  NumberType(real, rational or integer)
   b  -A
 end
kliem commented 4 years ago

Dependencies: #30293

kliem commented 4 years ago

Description changed:

--- 
+++ 
@@ -2,6 +2,7 @@

sage: P = polytopes.regular_polygon(5, exact=True) sage: P1 = polytopes.regular_polygon(5, exact=False) +sage: P*P1

KeyError Traceback (most recent call last) /srv/public/shared/sage-9.0/local/lib/python3.7/site-packages/sage/structure/coerce.pyx in sage.structure.coerce.CoercionModel.bin_op (build/cythonized/sage/structure/coerce.c:9946)()

kliem commented 4 years ago

Author: Jonathan Kliem

kliem commented 4 years ago

Branch: public/30328

kliem commented 4 years ago

Commit: 319c172

kliem commented 4 years ago

Description changed:

--- 
+++ 
@@ -3,122 +3,7 @@
 sage: P = polytopes.regular_polygon(5, exact=True)
 sage: P1 = polytopes.regular_polygon(5, exact=False)
 sage: P*P1
----------------------------------------------------------------------------
-KeyError                                  Traceback (most recent call last)
-/srv/public/shared/sage-9.0/local/lib/python3.7/site-packages/sage/structure/coerce.pyx in sage.structure.coerce.CoercionModel.bin_op (build/cythonized/sage/structure/coerce.c:9946)()
-   1195         try:
--> 1196             action = self._action_maps.get(xp, yp, op)
-   1197         except KeyError:
-
-/srv/public/shared/sage-9.0/local/lib/python3.7/site-packages/sage/structure/coerce_dict.pyx in sage.structure.coerce_dict.TripleDict.get (build/cythonized/sage/structure/coerce_dict.c:7917)()
-   1327         if not valid(cursor.key_id1):
--> 1328             raise KeyError((k1, k2, k3))
-   1329         value = <object>cursor.value
-
-KeyError: (Polyhedra in AA^2, Polyhedra in RDF^2, <built-in function mul>)
-
-During handling of the above exception, another exception occurred:
-
-ValueError                                Traceback (most recent call last)
-<ipython-input-4-69ee963b1fc0> in <module>()
-----> 1 P*P1
-
-/srv/public/shared/sage-9.0/local/lib/python3.7/site-packages/sage/structure/element.pyx in sage.structure.element.Element.__mul__ (build/cythonized/sage/structure/element.c:12029)()
-   1515             return (<Element>left)._mul_(right)
-   1516         if BOTH_ARE_ELEMENT(cl):
--> 1517             return coercion_model.bin_op(left, right, mul)
-   1518 
-   1519         cdef long value
-
-/srv/public/shared/sage-9.0/local/lib/python3.7/site-packages/sage/structure/coerce.pyx in sage.structure.coerce.CoercionModel.bin_op (build/cythonized/sage/structure/coerce.c:9996)()
-   1196             action = self._action_maps.get(xp, yp, op)
-   1197         except KeyError:
--> 1198             action = self.get_action(xp, yp, op, x, y)
-   1199         if action is not None:
-   1200             if (<Action>action)._is_left:
-
-/srv/public/shared/sage-9.0/local/lib/python3.7/site-packages/sage/structure/coerce.pyx in sage.structure.coerce.CoercionModel.get_action (build/cythonized/sage/structure/coerce.c:16783)()
-   1724         except KeyError:
-   1725             pass
--> 1726         action = self.discover_action(R, S, op, r, s)
-   1727         action = self.verify_action(action, R, S, op)
-   1728         self._action_maps.set(R, S, op, action)
-
-/srv/public/shared/sage-9.0/local/lib/python3.7/site-packages/sage/structure/coerce.pyx in sage.structure.coerce.CoercionModel.discover_action (build/cythonized/sage/structure/coerce.c:18201)()
-   1855         """
-   1856         if isinstance(R, Parent):
--> 1857             action = (<Parent>R).get_action(S, op, True, r, s)
-   1858             if action is not None:
-   1859                 return action
-
-/srv/public/shared/sage-9.0/local/lib/python3.7/site-packages/sage/structure/parent.pyx in sage.structure.parent.Parent.get_action (build/cythonized/sage/structure/parent.c:19745)()
-   2457         action = self._get_action_(S, op, self_on_left)
-   2458         if action is None:
--> 2459             action = self.discover_action(S, op, self_on_left, self_el, S_el)
-   2460 
-   2461         if action is not None:
-
-/srv/public/shared/sage-9.0/local/lib/python3.7/site-packages/sage/structure/parent.pyx in sage.structure.parent.Parent.discover_action (build/cythonized/sage/structure/parent.c:20722)()
-   2536                 # detect actions defined by _rmul_, _lmul_, _act_on_, and _acted_upon_ methods
-   2537                 from .coerce_actions import detect_element_action
--> 2538                 action = detect_element_action(self, S, self_on_left, self_el, S_el)
-   2539                 if action is not None:
-   2540                     return action
-
-/srv/public/shared/sage-9.0/local/lib/python3.7/site-packages/sage/structure/coerce_actions.pyx in sage.structure.coerce_actions.detect_element_action (build/cythonized/sage/structure/coerce_actions.c:5360)()
-    229     if isinstance(Y, Parent):
-    230         try:
---> 231             if x._acted_upon_(y, X_on_left) is not None:
-    232                 return ActedUponAction(Y, X, not X_on_left, False)
-    233         except CoercionException:
-
-/srv/public/shared/sage-9.0/local/lib/python3.7/site-packages/sage/structure/element.pyx in sage.structure.element.Element._acted_upon_ (build/cythonized/sage/structure/element.c:8604)()
-    925         return None
-    926 
---> 927     cpdef _acted_upon_(self, x, bint self_on_left):
-    928         """
-    929         Use this method to implement ``self`` acted on by x.
-
-/srv/public/shared/sage-9.0/local/lib/python3.7/site-packages/sage/geometry/polyhedron/base.py in _acted_upon_(self, actor, self_on_left)
-   4299         """
-   4300         if is_Polyhedron(actor):
--> 4301             return self.product(actor)
-   4302         if is_Vector(actor):
-   4303             return self.translation(actor)
-
-/srv/public/shared/sage-9.0/local/lib/python3.7/site-packages/sage/geometry/polyhedron/base.py in product(self, other)
-   3993 
-   3994         parent = self.parent().change_ring(new_ring, ambient_dim=self.ambient_dim() + other.ambient_dim())
--> 3995         return parent.element_class(parent, [new_vertices, new_rays, new_lines], None)
-   3996 
-   3997     _mul_ = product
-
-/srv/public/shared/sage-9.0/local/lib/python3.7/site-packages/sage/geometry/polyhedron/backend_cdd.py in __init__(self, parent, Vrep, Hrep, **kwds)
-    459             sage: TestSuite(p).run()
-    460         """
---> 461         Polyhedron_cdd.__init__(self, parent, Vrep, Hrep, **kwds)
-
-/srv/public/shared/sage-9.0/local/lib/python3.7/site-packages/sage/geometry/polyhedron/base.py in __init__(self, parent, Vrep, Hrep, **kwds)
-    120             vertices, rays, lines = Vrep
-    121             if vertices or rays or lines:
---> 122                 self._init_from_Vrepresentation(vertices, rays, lines, **kwds)
-    123             else:
-    124                 self._init_empty_polyhedron()
-
-/srv/public/shared/sage-9.0/local/lib/python3.7/site-packages/sage/geometry/polyhedron/backend_cdd.py in _init_from_Vrepresentation(self, vertices, rays, lines, verbose)
-     61         from .cdd_file_format import cdd_Vrepresentation
-     62         s = cdd_Vrepresentation(self._cdd_type, vertices, rays, lines)
----> 63         s = self._run_cdd(s, '--redcheck', verbose=verbose)
-     64         s = self._run_cdd(s, '--repall', verbose=verbose)
-     65         self._init_from_cdd_output(s)
-
-/srv/public/shared/sage-9.0/local/lib/python3.7/site-packages/sage/geometry/polyhedron/backend_cdd.py in _run_cdd(self, cdd_input_string, cmdline_arg, verbose)
-    163         if 'Error:' in ans + err:
-    164             # cdd reports errors on stdout and misc information on stderr
---> 165             raise ValueError(ans.strip())
-    166         return ans
-    167 
-
+...
 ValueError: *Input Error: Input format is not correct.
 *Format:
  begin
@@ -126,3 +11,9 @@
    b  -A
  end

+ +We fix this, by converting the values to reals, before passing them to cdd. + +We enable a doctest prepared by #30293, which implicitly checks that this is fixed. + +The above failure will be implicitly added by #29934.

kliem commented 4 years ago

New commits:

1f00faafix lawrence extension with base extension; add test method for lawrence construction
8e8158cfix missing conversion to RDF
319c172check lawrence construction with new vertex in RDF
mkoeppe commented 4 years ago
comment:7

Python 3.8 will complain about this; better to use == instead of is

+    if cdd_type is 'real':
kliem commented 4 years ago
comment:8

Replying to @mkoeppe:

Python 3.8 will complain about this; better to use == instead of is

+    if cdd_type is 'real':

Thanks. I had somewhere in the back of my mind, that one of them is wrong.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from 319c172 to 96adb63

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

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

96adb63python 3.8 compatible
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

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

2481e36avoid very long tests
f6d1327add some long time warnings
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from 96adb63 to f6d1327

mkoeppe commented 4 years ago

Reviewer: Matthias Koeppe

kliem commented 4 years ago
comment:12

Thank you.

kliem commented 4 years ago
comment:13

Needed to rebase on top of #30293 (which had a merge conflict and was rebased on top of the newest beta).


New commits:

eaeba9afix lawrence extension with base extension; add test method for lawrence construction
0c1a2daavoid very long tests
d7e07afadd some long time warnings
539930erun lawrence_polytope test for cdd and normaliz
04a92effix missing conversion to RDF
3cf080ccheck lawrence construction with new vertex in RDF
0be9ed3python 3.8 compatible
kliem commented 4 years ago

Changed commit from f6d1327 to 0be9ed3

kliem commented 4 years ago

Changed branch from public/30328 to public/30328-reb

vbraun commented 4 years ago

Changed branch from public/30328-reb to 0be9ed3