Ticket #6779 (closed defect: fixed)

Opened 4 years ago

Last modified 4 years ago

[with patch, positive review] positive_integer_relations bug in lattice_polytope

Reported by: novoselt Owned by: mhampton
Priority: major Milestone: sage-4.2.1
Component: geometry Keywords:
Cc: Work issues:
Report Upstream: Reviewers: Marshall Hampton
Authors: Andrey Novoseltsev Merged in:
Dependencies: Stopgaps:

Description

Since gcd(3/2, 9/5) used to be 3/10, it was used in lattice_polytope functions for rescaling to primitive integral vectors in the given rational direction. This is no longer true and leads to bugs: 

sage: p = ReflexivePolytope(2, 1)
sage: lattice_polytope.positive_integer_relations(p.vertices())
Traceback (most recent call last):
...
TypeError: matrix has denominators so can't change to ZZ.

The patch adds a function integral_length and uses it instead of gcd: 

sage: p = ReflexivePolytope(2, 1)
sage: lattice_polytope.positive_integer_relations(p.vertices())
[2 1 1]

Attachments

trac_6779_positive_integer_relations_bug_in_lattice_polytope.patch Download (2.7 KB) - added by novoselt 4 years ago.

Change History

Changed 4 years ago by novoselt

comment:1 Changed 4 years ago by mhampton

  • Status changed from needs_review to positive_review

Looks good, positive review.

comment:2 Changed 4 years ago by mhampton

  • Summary changed from [with patch, needs review] positive_integer_relations bug in lattice_polytope to [with patch, positive review] positive_integer_relations bug in lattice_polytope

comment:3 Changed 4 years ago by novoselt

This patch is included as a part of a rebased patch for #6831.

comment:4 Changed 4 years ago by mhansen

  • Status changed from positive_review to closed
  • Reviewers set to Marshall Hampton
  • Resolution set to fixed
  • Authors set to Andrey Novoseltsev

Fixed as part of #6831

Note: See TracTickets for help on using tickets.