Opened 2 years ago
Closed 2 years ago
#30328 closed defect (fixed)
Need to convert values before passing them to `cdd`
Reported by:  ghkliem  Owned by:  

Priority:  critical  Milestone:  sage9.2 
Component:  geometry  Keywords:  geometry, coercion 
Cc:  jipilab, ghLaisRast  Merged in:  
Authors:  Jonathan Kliem  Reviewers:  Matthias Koeppe 
Report Upstream:  N/A  Work issues:  
Branch:  0be9ed3 (Commits, GitHub, GitLab)  Commit:  0be9ed32eb667a4e3bf8b7d460dfa5c7cfaad760 
Dependencies:  #30293  Stopgaps: 
Description (last modified by )
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 2 years ago by
 Keywords geometry coercion added
comment:2 Changed 2 years ago by
comment:3 Changed 2 years ago by
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) <ipythoninput143a950b1911b2> in <module>() > 1 tfcell.lawrence_extension(tfcell.vertices()[Integer(0)].vector()*RealNumber('1.0')) /home/jonathan/Applications/sage/local/lib/python3.7/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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 2 years ago by
 Dependencies set to #30293
 Description modified (diff)
comment:5 Changed 2 years ago by
 Branch set to public/30328
 Commit set to 319c172084163f0dce96d90a1762878d7c3a741e
 Description modified (diff)
 Status changed from new to needs_review
comment:6 Changed 2 years ago by
 Summary changed from Coercion of polyhedra over `RDF` and `AA` fails. to Need to convert values before passing them to `cdd`
comment:7 followup: ↓ 8 Changed 2 years ago by
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 2 years ago by
Replying to mkoeppe:
Python 3.8 will complain about this; better to use
==
instead ofis
+ if cdd_type is 'real':
Thanks. I had somewhere in the back of my mind, that one of them is wrong.
comment:9 Changed 2 years ago by
 Commit changed from 319c172084163f0dce96d90a1762878d7c3a741e to 96adb63b2430ff5b6b195e7b3132691dd5bfde4d
Branch pushed to git repo; I updated commit sha1. New commits:
96adb63  python 3.8 compatible

comment:10 Changed 2 years ago by
 Commit changed from 96adb63b2430ff5b6b195e7b3132691dd5bfde4d to f6d1327a6fe5f217d01043a845c6c6c3a8a77743
comment:11 Changed 2 years ago by
 Priority changed from major to critical
 Reviewers set to Matthias Koeppe
 Status changed from needs_review to positive_review
comment:12 Changed 2 years ago by
Thank you.
comment:13 Changed 2 years ago by
 Branch changed from public/30328 to public/30328reb
 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:
eaeba9a  fix lawrence extension with base extension; add test method for lawrence construction

0c1a2da  avoid very long tests

d7e07af  add some long time warnings

539930e  run lawrence_polytope test for cdd and normaliz

04a92ef  fix missing conversion to RDF

3cf080c  check lawrence construction with new vertex in RDF

0be9ed3  python 3.8 compatible

comment:14 Changed 2 years ago by
 Branch changed from public/30328reb to 0be9ed32eb667a4e3bf8b7d460dfa5c7cfaad760
 Resolution set to fixed
 Status changed from positive_review to closed
This was detected in #29934.