Opened 3 years ago
Closed 3 years ago
#29073 closed defect (fixed)
`gale_transform` does not work for `RDF`
Reported by:  ghkliem  Owned by:  

Priority:  major  Milestone:  sage9.1 
Component:  geometry  Keywords:  polytopes, gale transform, RDF 
Cc:  jipilab, ghLaisRast  Merged in:  
Authors:  Jonathan Kliem  Reviewers:  Laith Rastanawi 
Report Upstream:  N/A  Work issues:  
Branch:  9683151 (Commits, GitHub, GitLab)  Commit:  96831516e18b4ecb50bf2ecf4e18f7056c9edd78 
Dependencies:  Stopgaps: 
Description
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.43689570931383e16, 1.6653345369377348e16, 6.661338147750939e16, 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.
Change History (7)
comment:1 Changed 3 years ago by
 Branch set to public/29073
 Commit set to 29f9ebe6502ded976393f20fc5bcb1c2d5b846e8
 Status changed from new to needs_review
comment:2 Changed 3 years ago by
 Commit changed from 29f9ebe6502ded976393f20fc5bcb1c2d5b846e8 to cfd9b1a1d2aac511019d51068aa57921e5686703
Branch pushed to git repo; I updated commit sha1. New commits:
cfd9b1a  fixed doc indent

comment:3 Changed 3 years ago by
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.
comment:4 Changed 3 years ago by
 Commit changed from cfd9b1a1d2aac511019d51068aa57921e5686703 to 96831516e18b4ecb50bf2ecf4e18f7056c9edd78
Branch pushed to git repo; I updated commit sha1. New commits:
9683151  better comment in doc of left_kernel

comment:5 Changed 3 years ago by
 Status changed from needs_review to positive_review
comment:6 Changed 3 years ago by
 Reviewers set to Laith Rastanawi
comment:7 Changed 3 years ago by
 Branch changed from public/29073 to 96831516e18b4ecb50bf2ecf4e18f7056c9edd78
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
add notes about not echolonizing kernels for inexact rings
do not echolonize kernel basis for gale transpose