Opened 13 months ago
Closed 12 months ago
#30146 closed defect (fixed)
#29843 introduces a bug in Polyhedron().linear_transformation
Reported by:  etn40ff  Owned by:  

Priority:  critical  Milestone:  sage9.2 
Component:  geometry  Keywords:  combinatorial polyhedron, linear transformation 
Cc:  jipilab, ghLaisRast, ghkliem, mkoeppe  Merged in:  
Authors:  Jonathan Kliem  Reviewers:  Matthias Koeppe 
Report Upstream:  N/A  Work issues:  
Branch:  c9c7b63 (Commits, GitHub, GitLab)  Commit:  c9c7b63f1ed9fda028044385920f2b7849cfbd2b 
Dependencies:  Stopgaps: 
Description
In 9.2.beta5 applying linear transformations to a polyhedron containing a ray is broken:
sage: Polyhedron(rays=[(0,1)]).linear_transformation(identity_matrix(2))  ValueError Traceback (most recent call last) <ipythoninput1853c5f2df4fe7c> in <module>() > 1 Polyhedron(rays=[(Integer(0),Integer(1))]).linear_transformation(identity_matrix(Integer(2))) /opt/sage/sage/local/lib/python3.7/sitepackages/sage/geometry/polyhedron/base.py in linear_transformation(self, linear_transf, new_base_ring) 5093 5094 sage: P = Polyhedron([(0,0),(1,1)], base_ring=ZZ) > 5095 sage: P.intersection(P) 5096 A 1dimensional polyhedron in ZZ^2 defined as the convex hull of 2 vertices 5097 sage: Q = Polyhedron([(0,1),(1,0)], base_ring=ZZ) /opt/sage/sage/local/lib/python3.7/sitepackages/sage/geometry/polyhedron/base.py in __init__(self, parent, Vrep, Hrep, Vrep_minimal, Hrep_minimal, pref_rep, **kwds) 218 # TODO: find something better *but* fast 219 return hash((self.dim(), > 220 self.ambient_dim(), 221 self.n_Hrepresentation(), 222 self.n_Vrepresentation(), /opt/sage/sage/local/lib/python3.7/sitepackages/sage/geometry/polyhedron/backend_ppl.py in _init_from_Vrepresentation(self, vertices, rays, lines, minimize, verbose) 75 d = LCM_list([denominator(r_i) for r_i in r]) 76 if d.is_one(): > 77 gs.insert(ray(Linear_Expression(r, 0))) 78 else: 79 dr = [ d*r_i for r_i in r ] ppl/generator.pyx in ppl.generator.Generator.ray() ppl/generator.pyx in ppl.generator.Generator.ray() ValueError: PPL::ray(e): e == 0, but the origin cannot be a ray.
this used to work just fine in 9.2.beta4.
git bisect
seems to blame the regression on #29843
Change History (9)
comment:1 Changed 13 months ago by
 Priority changed from major to critical
comment:2 Changed 13 months ago by
comment:3 Changed 13 months ago by
 Branch set to public/30146
 Commit set to 4eff4137dfac9c656bb8e50343be76efae2974ae
 Status changed from new to needs_review
comment:4 followup: ↓ 5 Changed 13 months ago by
 Commit changed from 4eff4137dfac9c656bb8e50343be76efae2974ae to 7fe6b43d2ab3adde507fe745726b82c3bbc8059d
Branch pushed to git repo; I updated commit sha1. New commits:
7fe6b43  One transposition only

comment:5 in reply to: ↑ 4 Changed 13 months ago by
Replying to git:
Branch pushed to git repo; I updated commit sha1. New commits:
7fe6b43 One transposition only
I was thinking about that one. At first glance this might be a bit weird, because the matrix is supposed to act from the left. However it is shorter and easier to read I guess (and slightly faster I suppose).
comment:6 Changed 13 months ago by
 Commit changed from 7fe6b43d2ab3adde507fe745726b82c3bbc8059d to c9c7b63f1ed9fda028044385920f2b7849cfbd2b
Branch pushed to git repo; I updated commit sha1. New commits:
c9c7b63  missed preceeding `sage:` of doctests

comment:7 Changed 12 months ago by
 Reviewers set to Matthias Koeppe
 Status changed from needs_review to positive_review
comment:8 Changed 12 months ago by
Thank you.
comment:9 Changed 12 months ago by
 Branch changed from public/30146 to c9c7b63f1ed9fda028044385920f2b7849cfbd2b
 Resolution set to fixed
 Status changed from positive_review to closed
Thanks for catching this. This is another friendly reminder that working on those test suites is really useful.