Opened 3 years ago

Closed 3 years ago

#23355 closed defect (fixed)

affine hull of one point polyhedron

Reported by: moritz Owned by:
Priority: minor Milestone: sage-8.0
Component: geometry Keywords: polyhedron
Cc: vdelecroix, jipilab Merged in:
Authors: Moritz Firsching Reviewers: Jean-Philippe Labbé
Report Upstream: N/A Work issues:
Branch: 9fccf4e (Commits) Commit: 9fccf4ed8fd21d8500ad78cbcfac142847cffccd
Dependencies: #22804 Stopgaps:

Description

When using the orthogonal/orthonormal feature of the affine hull function of a polyhedron (introduced in #22804), there is a problem with the one point polyhedron:

sage: P = Polyhedron([[7]])
sage: P
A 0-dimensional polyhedron in ZZ^1 defined as the convex hull of 1 vertex
sage: P.affine_hull()
A 0-dimensional polyhedron in ZZ^0 defined as the convex hull of 1 verte
sage: P.affine_hull(orthogonal='True')
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
...
TypeError: unsupported operand parent(s) for *: 'Full MatrixSpace of 0 by 0 dense matrices over Integer Ring' and 'Ambient free module of rank 1 over the principal ideal domain Integer Ring'
sage: P.affine_hull(orthonormal='True')
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
...
TypeError: unsupported operand parent(s) for *: 'Full MatrixSpace of 0 by 0 dense matrices over Integer Ring' and 'Ambient free module of rank 1 over the principal ideal domain Integer Ring'

This is how it should be:

sage: P = Polyhedron([[7]]); P
A 0-dimensional polyhedron in ZZ^1 defined as the convex hull of 1 vertex
sage: P.affine_hull()
A 0-dimensional polyhedron in ZZ^0 defined as the convex hull of 1 vertex
sage: P.affine_hull(orthonormal='True')
A 0-dimensional polyhedron in QQ^0 defined as the convex hull of 1 vertex
sage: P.affine_hull(orthogonal='True')
A 0-dimensional polyhedron in QQ^0 defined as the convex hull of 1 vertex

Change History (7)

comment:1 Changed 3 years ago by moritz

  • Branch set to u/moritz/fix_one_point_affine_hull

comment:2 Changed 3 years ago by moritz

  • Cc vdelecroix added
  • Commit set to 08d951f6f1cffa278948a7913360c3999407670a
  • Status changed from new to needs_review

The reason why this didn't work properly is the fact that if a Matrix is generated from a list it cannot correctly guess its dimensions if the list is empty. The whole fix is simply to exchange the line

M = matrix(self.base_ring(), [list(w) for w in itertools.islice(v.neighbors(), self.dim())])

by

M = matrix(self.base_ring(), self.dim(), self.ambient_dim(), [list(w) for w in itertools.islice(v.neighbors(), self.dim())])

giving the dimensions explicitly. I also added a doctest (under the section TESTS) documenting that the bug is fixed.


New commits:

029246dprescribe size of matrix explicitely
08d951fadded hidden doctest

comment:3 Changed 3 years ago by jipilab

Hi Moritz,

Could you add a reference to this ticket just before the tests you added?

Apart from that it looks good to me! You can set it to positive review on my behalf once this is done!

comment:4 Changed 3 years ago by jipilab

  • Cc jipilab added
  • Reviewers set to Jean-Philippe Labbé

comment:5 Changed 3 years ago by git

  • Commit changed from 08d951f6f1cffa278948a7913360c3999407670a to 9fccf4ed8fd21d8500ad78cbcfac142847cffccd

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

9fccf4eadded trac number in test

comment:6 Changed 3 years ago by moritz

  • Status changed from needs_review to positive_review

thanks for the review, JP!

I added the desired reference and will put it on 'positive review' as you suggested

comment:7 Changed 3 years ago by vbraun

  • Branch changed from u/moritz/fix_one_point_affine_hull to 9fccf4ed8fd21d8500ad78cbcfac142847cffccd
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.