Opened 3 years ago
Closed 3 years ago
#28377 closed defect (fixed)
polymake interface broken with "nonstandard" quadratic fields
Reported by:  vdelecroix  Owned by:  

Priority:  major  Milestone:  sage8.9 
Component:  geometry  Keywords:  
Cc:  mkoeppe, slabbe, SimonKing, jipilab  Merged in:  
Authors:  Vincent Delecroix  Reviewers:  JeanPhilippe Labbé 
Report Upstream:  N/A  Work issues:  
Branch:  8cb0d7e (Commits, GitHub, GitLab)  Commit:  8cb0d7e828698f1de43b356fad35b6a466b401ff 
Dependencies:  Stopgaps: 
Description (last modified by )
The polymake interface is confused with Sage number fields that are not of the form QQ[sqrt(D)]
. Here is a simple example
sage: x = polygen(QQ, 'x') sage: K = NumberField(x^2  x 1, 'a', embedding=(1AA(5).sqrt())/2) sage: a = K.gen() sage: L = NumberField(x^2  5, 'b', embedding=AA(5).sqrt()) sage: b = L.gen() sage: polymake(a) 0+1r5 sage: polymake(b) 0+1r5 sage: polymake(a) * polymake(a) 5 sage: polymake(b) * polymake(b) 5
As a consequence, constructing polytopes with polymake backend is completely broken
sage: f = Hom(K, L)([(1b)/2]) sage: Kverts = [(0,0,a),(1,a,2a),(2*a,3,1),(2a,5+2*a,3*a7),(3,2*a1,1),(1,a,a)] sage: Lverts = [list(map(f, v)) for v in Kverts] sage: Polyhedron(Kverts, base_ring=K, backend='polymake').f_vector() (1, 6, 12, 8, 1) sage: Polyhedron(Lverts, base_ring=L, backend='polymake').f_vector() (1, 5, 9, 6, 1)
One can check that it is the second one which is correct
sage: Polyhedron(Kverts, base_ring=K, backend='field').f_vector() (1, 5, 9, 6, 1) sage: Polyhedron(Lverts, base_ring=L, backend='field').f_vector() (1, 5, 9, 6, 1)
Note: This is tested with the jupymake
interface but it should not matter (?)
Change History (11)
comment:1 Changed 3 years ago by
 Description modified (diff)
comment:2 Changed 3 years ago by
 Cc SimonKing added
comment:3 Changed 3 years ago by
 Description modified (diff)
comment:4 Changed 3 years ago by
 Description modified (diff)
comment:5 Changed 3 years ago by
 Cc jipilab added
comment:6 Changed 3 years ago by
 Branch set to u/vdelecroix/28377
 Commit set to ab4e664f3a8dcff23f55df0a055c692d7cfe6477
 Status changed from new to needs_review
no comment on the previous implementation!!
New commits:
ab4e664  fix conversion of quadratic nf elt to polymake

comment:7 Changed 3 years ago by
 Commit changed from ab4e664f3a8dcff23f55df0a055c692d7cfe6477 to 8cb0d7e828698f1de43b356fad35b6a466b401ff
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
8cb0d7e  fix conversion of quadratic nf elt to polymake

comment:8 followup: ↓ 9 Changed 3 years ago by
 Reviewers set to JeanPhilippe Labbé
Looks good to me.
I am hesitating to put a test on one of the observed wrong fvector, but if you confirm that this fixes the observed symptom on the fvector, then I would say it is fine.
comment:9 in reply to: ↑ 8 ; followup: ↓ 10 Changed 3 years ago by
Replying to jipilab:
Looks good to me.
I am hesitating to put a test on one of the observed wrong fvector, but if you confirm that this fixes the observed symptom on the fvector, then I would say it is fine.
This ticket has nothing to do with polytopes :) If you feel safer having examples involving polytope I let you add one.
comment:10 in reply to: ↑ 9 Changed 3 years ago by
 Status changed from needs_review to positive_review
Replying to vdelecroix:
Replying to jipilab:
Looks good to me.
I am hesitating to put a test on one of the observed wrong fvector, but if you confirm that this fixes the observed symptom on the fvector, then I would say it is fine.
This ticket has nothing to do with polytopes :) If you feel safer having examples involving polytope I let you add one.
After looking carefully at the branch, I saw what you meant. I got confused by the description of the ticket...
It's all good.
comment:11 Changed 3 years ago by
 Branch changed from u/vdelecroix/28377 to 8cb0d7e828698f1de43b356fad35b6a466b401ff
 Resolution set to fixed
 Status changed from positive_review to closed
Je n'ai pas ajouté le bon Labbé! Désolé Sébastien pour le spam.