Opened 3 years ago

Closed 3 years ago

#28377 closed defect (fixed)

polymake interface broken with "non-standard" quadratic fields

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-8.9
Component: geometry Keywords:
Cc: mkoeppe, slabbe, SimonKing, jipilab Merged in:
Authors: Vincent Delecroix Reviewers: Jean-Philippe Labbé
Report Upstream: N/A Work issues:
Branch: 8cb0d7e (Commits, GitHub, GitLab) Commit: 8cb0d7e828698f1de43b356fad35b6a466b401ff
Dependencies: Stopgaps:

Status badges

Description (last modified by vdelecroix)

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=(1-AA(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)([(1-b)/2])
sage: Kverts = [(0,0,a),(1,a,2-a),(2*a,3,1),(2-a,5+2*a,3*a-7),(-3,2*a-1,-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 vdelecroix

  • Description modified (diff)

comment:2 Changed 3 years ago by vdelecroix

  • Cc SimonKing added

comment:3 Changed 3 years ago by vdelecroix

  • Description modified (diff)

comment:4 Changed 3 years ago by vdelecroix

  • Description modified (diff)

comment:5 Changed 3 years ago by vdelecroix

  • Cc jipilab added

Je n'ai pas ajouté le bon Labbé! Désolé Sébastien pour le spam.

comment:6 Changed 3 years ago by vdelecroix

  • Authors set to Vincent Delecroix
  • 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:

ab4e664fix conversion of quadratic nf elt to polymake

comment:7 Changed 3 years ago by git

  • Commit changed from ab4e664f3a8dcff23f55df0a055c692d7cfe6477 to 8cb0d7e828698f1de43b356fad35b6a466b401ff

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

8cb0d7efix conversion of quadratic nf elt to polymake

comment:8 follow-up: Changed 3 years ago by jipilab

  • Reviewers set to Jean-Philippe Labbé

Looks good to me.

I am hesitating to put a test on one of the observed wrong f-vector, but if you confirm that this fixes the observed symptom on the f-vector, then I would say it is fine.

comment:9 in reply to: ↑ 8 ; follow-up: Changed 3 years ago by vdelecroix

Replying to jipilab:

Looks good to me.

I am hesitating to put a test on one of the observed wrong f-vector, but if you confirm that this fixes the observed symptom on the f-vector, 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 jipilab

  • 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 f-vector, but if you confirm that this fixes the observed symptom on the f-vector, 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 vbraun

  • Branch changed from u/vdelecroix/28377 to 8cb0d7e828698f1de43b356fad35b6a466b401ff
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.