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

gale_transform does not work for RDF #29073

Closed
kliem opened this issue Jan 23, 2020 · 11 comments
Closed

gale_transform does not work for RDF #29073

kliem opened this issue Jan 23, 2020 · 11 comments

Comments

@kliem
Copy link
Contributor

kliem commented Jan 23, 2020

Currently the Gale transform for RDF does not work

sage: P = polytopes.icosahedron(exact=False)
sage: sum(P.gale_transform())
(1.3819660112000005, -1.3819660112000005, 0.3819660112000003, -1.3819660112000005, -9.43689570931383e-16, 1.6653345369377348e-16, -6.661338147750939e-16, 0.0)

but the sum should be close to zero. This is of course only an indication that something goes terribly wrong. For #29065, I wanted to add a doctest recovering an inexact polyhedron from the gale diagram, but it simply does not work.

The problem is quite simple. The method right_kernel for matrices, should just not be used, as it echolonizes the kernel. This is just something, one shouldn't do for inexact rings. The method right_kernel_matrix(basis=computed) gives a much more useful matrix.

We also leave a note in left_kernel, right_kernel, and right_kernel_matrix to indicate this problem.

CC: @jplab @LaisRast

Component: geometry

Keywords: polytopes, gale transform, RDF

Author: Jonathan Kliem

Branch/Commit: 9683151

Reviewer: Laith Rastanawi

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

@kliem kliem added this to the sage-9.1 milestone Jan 23, 2020
@kliem
Copy link
Contributor Author

kliem commented Jan 23, 2020

Branch: public/29073

@kliem
Copy link
Contributor Author

kliem commented Jan 23, 2020

Commit: 29f9ebe

@kliem
Copy link
Contributor Author

kliem commented Jan 23, 2020

New commits:

1db3349add notes about not echolonizing kernels for inexact rings
29f9ebedo not echolonize kernel basis for gale transpose

@kliem
Copy link
Contributor Author

kliem commented Jan 23, 2020

Author: Jonathan Kliem

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 27, 2020

Changed commit from 29f9ebe to cfd9b1a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 27, 2020

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

cfd9b1afixed doc indent

@LaisRast
Copy link

comment:3

In left_kernel, explain your note a little bit:

             For inexact rings use :meth:`right_kernel_matrix` with
-            ``basis='computed'`` to avoid echolonizing.
+            ``basis='computed'`` (on the transpose of the matrix) to avoid echolonizing.

Otherwise, it is good to go.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 30, 2020

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

9683151better comment in doc of left_kernel

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 30, 2020

Changed commit from cfd9b1a to 9683151

@LaisRast
Copy link

Reviewer: Laith Rastanawi

@vbraun
Copy link
Member

vbraun commented Jan 31, 2020

Changed branch from public/29073 to 9683151

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