Polyhedron.affine_hull() raises AssertionError

[yzh]

The method affine_hull of Polyhedron fails.

sage: P1 = Polyhedron(vertices=([[-1, 1], [0, -1], [0, 0], [-1, -1]]))
sage: P2 = Polyhedron(vertices=[[1, 1], [1, -1], [0, -1], [0, 0]])
sage: P = P1.intersection(P2)
sage: A, b = P.affine_hull(as_affine_map=True, orthonormal=True, extend=True)
AssertionError

This is because the order of the vertices of P is changed by the translation by zero vector.

sage: P.vertices()
(A vertex at (0, 0), A vertex at (0, -1))
sage: P.translation(vector([0,0])).vertices()
(A vertex at (0, -1), A vertex at (0, 0))

[vdelecroix]

1) It would be bettter to sort the V and H generators so that this kind of troubles never appear (and equality test would be even faster)

2) The translation method is stupidly written

    displacement = vector(displacement)
    new_vertices = [x.vector()+displacement for x in self.vertex_generator()]
    new_rays = self.rays()
    new_lines = self.lines()
    new_ring = self.parent()._coerce_base_ring(displacement)
    return Polyhedron(vertices=new_vertices, rays=new_rays, lines=new_lines, base_ring=new_ring)

we should not recompute anything!

[mkoeppe]

Replying to vdelecroix:

we should not recompute anything!

That would require #22701 - Setting up a Polyhedron from both Vrep and Hrep.

[jipilab]

@Moritz: You told me you would have a fix for this? It consists in what exactly?

[moritz]

This should fix at least the reported bug; it simply chooses the zero, instead of relying on the fact that it is the first in the list.

---

New commits:

​ffd2bdd	compensate the order change of 'translation'
​0022adc	added test from trac

[tscrim]

You have some tab characters (should be 4 spaces) and your indentation level is wrong:

       Check that :trac:`24047` is fixed::
 
            sage: P1 = Polyhedron(vertices=([[-1, 1], [0, -1], [0, 0], [-1, -1]]))
            sage: P2 = Polyhedron(vertices=[[1, 1], [1, -1], [0, -1], [0, 0]])
            sage: P = P1.intersection(P2)
            sage: A, b = P.affine_hull(as_affine_map=True, orthonormal=True, extend=True)

[git]

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

​0bee595

tabs to spaces

[moritz]

uups, I used a different editor, which changed spaces by tabs.. Should be fixed now.

[jipilab]

that we really Q really -> that Q really?

Otherwise, it looks good to me and the bot is happy so I would put it as positive review once the small mistake above is corrected.

Thanks for the quick fix!

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​7036f1c	compensate the order change of 'translation'
​d4456e3	added test from trac
​e081292	tabs to spaces
​70114be	typo in comment

[moritz]

thanks for the review, JP!

[chapoton]

reviewer name, please