Opened 2 years ago

Closed 2 years ago

#30293 closed defect (fixed)

bug in lawrence_extension

Reported by: gh-LaisRast Owned by:
Priority: major Milestone: sage-9.2
Component: geometry Keywords: polytope, lawrence_extension
Cc: jipilab, gh-kliem Merged in:
Authors: Jonathan Kliem Reviewers: Laith Rastanawi
Report Upstream: N/A Work issues:
Branch: 539930e (Commits, GitHub, GitLab) Commit: 539930e4d10bbd9050065b06203f60d6c5963ef1
Dependencies: Stopgaps:

Status badges

Description (last modified by gh-kliem)

The method lawrence_extension of Polyhedron does not work when the point at which we do the lawrence_extension has a different base_ring than the Polyhedron: 

sage: polytopes.cube().lawrence_extension([5/2,0,0])
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
...
TypeError: no conversion of this rational to integer

The bug was introduced in #27926.

We fix this and add a method _test_lawrence to systematically test the lawrence construction.

Change History (10)

comment:1 Changed 2 years ago by gh-kliem 

-parent = self.parent().change_ring(self.base_ring(), ambient_dim = self.ambient_dim() +  1)
+parent = self.parent().base_extend(v, ambient_dim=self.ambient_dim() +  1)

I think this should to the job.

And this ticket shows again that we should test more methods.

comment:2 Changed 2 years ago by gh-kliem

  • Authors set to Jonathan Kliem
  • Branch set to public/30293
  • Commit set to 1f00faa1c3f29641675d844cd899c680c367cf65
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

​1f00faafix lawrence extension with base extension; add test method for lawrence construction

comment:3 Changed 2 years ago by git

  • Commit changed from 1f00faa1c3f29641675d844cd899c680c367cf65 to b831f3f744b0d9e9bf56d52e153f272ded3290b2

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

​740a654avoid very long tests
​b831f3fadd some long time warnings

comment:4 Changed 2 years ago by git

  • Commit changed from b831f3f744b0d9e9bf56d52e153f272ded3290b2 to a96a977cb37b51c3cb6141772546b2515b48019d

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

​a96a977run lawrence_polytope test for cdd and normaliz

comment:5 Changed 2 years ago by gh-LaisRast

  • Reviewers set to Laith Rastanawi
  • Status changed from needs_review to positive_review

It looks good to me.

comment:6 Changed 2 years ago by gh-kliem

Thank you.

comment:7 Changed 2 years ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:8 Changed 2 years ago by gh-kliem

This is the merge conflict, which has an obvious solution. 

++<<<<<<< HEAD
 +            sage: TestSuite(id).run(skip=["_test_is_combinatorially_isomorphic", "_test_pyramid"])
++=======
+             sage: TestSuite(id).run(skip=["_test_is_combinatorially_isomorphic", "_test_lawrence"])
++>>>>>>> 1f00faa1c3... fix lawrence extension with base extension; add test method for lawrence construction

comment:9 Changed 2 years ago by gh-kliem

  • Branch changed from public/30293 to public/30293-reb
  • Commit changed from a96a977cb37b51c3cb6141772546b2515b48019d to 539930e4d10bbd9050065b06203f60d6c5963ef1
  • Status changed from needs_work to positive_review

Trivial merge conflict.

New commits:

​eaeba9afix lawrence extension with base extension; add test method for lawrence construction
​0c1a2daavoid very long tests
​d7e07afadd some long time warnings
​539930erun lawrence_polytope test for cdd and normaliz

comment:10 Changed 2 years ago by vbraun

  • Branch changed from public/30293-reb to 539930e4d10bbd9050065b06203f60d6c5963ef1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.