Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug in lawrence_extension #30293

Closed
LaisRast opened this issue Aug 5, 2020 · 19 comments
Closed

bug in lawrence_extension #30293

LaisRast opened this issue Aug 5, 2020 · 19 comments

Comments

@LaisRast
Copy link
Contributor

LaisRast commented Aug 5, 2020

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.

CC: @jplab @kliem

Component: geometry

Keywords: polytope, lawrence_extension

Author: Jonathan Kliem

Branch/Commit: 539930e

Reviewer: Laith Rastanawi

Issue created by migration from https://trac.sagemath.org/ticket/30293

@LaisRast LaisRast added this to the sage-9.2 milestone Aug 5, 2020
@kliem
Copy link
Contributor

kliem commented Aug 5, 2020

comment:1
-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.

@kliem
Copy link
Contributor

kliem commented Aug 10, 2020

Author: Jonathan Kliem

@kliem
Copy link
Contributor

kliem commented Aug 10, 2020

New commits:

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

@kliem
Copy link
Contributor

kliem commented Aug 10, 2020

Commit: 1f00faa

@kliem
Copy link
Contributor

kliem commented Aug 10, 2020

Branch: public/30293

@kliem

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 10, 2020

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

740a654avoid very long tests
b831f3fadd some long time warnings

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 10, 2020

Changed commit from 1f00faa to b831f3f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 10, 2020

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

a96a977run lawrence_polytope test for cdd and normaliz

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 10, 2020

Changed commit from b831f3f to a96a977

@LaisRast
Copy link
Contributor Author

comment:5

It looks good to me.

@LaisRast
Copy link
Contributor Author

Reviewer: Laith Rastanawi

@kliem
Copy link
Contributor

kliem commented Aug 18, 2020

comment:6

Thank you.

@vbraun
Copy link
Member

vbraun commented Aug 18, 2020

comment:7

Merge conflict

@kliem
Copy link
Contributor

kliem commented Aug 19, 2020

comment:8

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

@kliem
Copy link
Contributor

kliem commented Aug 19, 2020

Changed commit from a96a977 to 539930e

@kliem
Copy link
Contributor

kliem commented Aug 19, 2020

comment:9

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

@kliem
Copy link
Contributor

kliem commented Aug 19, 2020

Changed branch from public/30293 to public/30293-reb

@vbraun
Copy link
Member

vbraun commented Aug 23, 2020

Changed branch from public/30293-reb to 539930e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants