Opened 8 months ago

Closed 8 months ago

#30328 closed defect (fixed)

Need to convert values before passing them to `cdd`

Reported by: gh-kliem Owned by:
Priority: critical Milestone: sage-9.2
Component: geometry Keywords: geometry, coercion
Cc: jipilab, gh-LaisRast Merged in:
Authors: Jonathan Kliem Reviewers: Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: 0be9ed3 (Commits, GitHub, GitLab) Commit: 0be9ed32eb667a4e3bf8b7d460dfa5c7cfaad760
Dependencies: #30293 Stopgaps:

Status badges

Description (last modified by gh-kliem)

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.

Change History (14)

comment:1 Changed 8 months ago by gh-kliem

  • Keywords geometry coercion added

comment:2 Changed 8 months ago by gh-kliem

This was detected in #29934.

comment:3 Changed 8 months ago by gh-kliem

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

comment:4 Changed 8 months ago by gh-kliem

  • Dependencies set to #30293
  • Description modified (diff)

comment:5 Changed 8 months ago by gh-kliem

  • Authors set to Jonathan Kliem
  • Branch set to public/30328
  • Commit set to 319c172084163f0dce96d90a1762878d7c3a741e
  • Description modified (diff)
  • Status changed from new to needs_review

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

comment:6 Changed 8 months ago by gh-kliem

  • Summary changed from Coercion of polyhedra over `RDF` and `AA` fails. to Need to convert values before passing them to `cdd`

comment:7 follow-up: Changed 8 months ago by mkoeppe

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

+    if cdd_type is 'real':

comment:8 in reply to: ↑ 7 Changed 8 months ago by gh-kliem

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.

comment:9 Changed 8 months ago by git

  • Commit changed from 319c172084163f0dce96d90a1762878d7c3a741e to 96adb63b2430ff5b6b195e7b3132691dd5bfde4d

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

96adb63python 3.8 compatible

comment:10 Changed 8 months ago by git

  • Commit changed from 96adb63b2430ff5b6b195e7b3132691dd5bfde4d to f6d1327a6fe5f217d01043a845c6c6c3a8a77743

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

2481e36avoid very long tests
f6d1327add some long time warnings

comment:11 Changed 8 months ago by mkoeppe

  • Priority changed from major to critical
  • Reviewers set to Matthias Koeppe
  • Status changed from needs_review to positive_review

comment:12 Changed 8 months ago by gh-kliem

Thank you.

comment:13 Changed 8 months ago by gh-kliem

  • Branch changed from public/30328 to public/30328-reb
  • Commit changed from f6d1327a6fe5f217d01043a845c6c6c3a8a77743 to 0be9ed32eb667a4e3bf8b7d460dfa5c7cfaad760

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

comment:14 Changed 8 months ago by vbraun

  • Branch changed from public/30328-reb to 0be9ed32eb667a4e3bf8b7d460dfa5c7cfaad760
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.