sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 482 forks source link

Polyhedra coercion of base rings fails for number fields #28770

Closed kliem closed 4 years ago

kliem commented 4 years ago

Currently coercion of polyhedra with number fields does not work. The following results in a type error:

sage: z = QQ['z'].0                                          
sage: K = NumberField(z^2 - 2,'s')
sage: K.gen()*polytopes.simplex(backend='field')

But the backend can surely handle it, as the following does work:

sage: z = QQ['z'].0                                          
sage: K = NumberField(z^2 - 2,'s')
sage: K.gen()*polytopes.simplex(backend='field', base_ring=K)
A 3-dimensional polyhedron in (Number Field in s with defining polynomial z^2 - 2)^4 defined as the convex hull of 4 vertices

The underlying error:

sage: z = QQ['z'].0                                          
sage: K = NumberField(z^2 - 2,'s')
sage: parent = polytopes.simplex().parent()
sage: parent._coerce_base_ring(K)
Rational Field

But it should be K.

The problem is that _coerce_base_ring of Polyhedra_base just takes the base ring of K, which are the rationals.

We fix this, by not taking the base ring, if the object is already a ring.

CC: @jplab @LaisRast

Component: geometry

Author: Jonathan Kliem

Branch/Commit: a0068cc

Reviewer: Léo Brunswic

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

kliem commented 4 years ago

Description changed:

--- 
+++ 
@@ -15,6 +15,18 @@
 A 3-dimensional polyhedron in (Number Field in s with defining polynomial z^2 - 2)^4 defined as the convex hull of 4 vertices

-The problem is that _coerce_base_ring of Polyhedra just takes the base ring of K, which are the rationals. +The underlying error: + + +sage: z = QQ['z'].0 +sage: K = NumberField(z^2 - 2,'s') +sage: parent = polytopes.simplex().parent() +sage: parent._coerce_base_ring(K) +Rational Field + + +But it should be K. + +The problem is that _coerce_base_ring of Polyhedra_base just takes the base ring of K, which are the rationals.

We fix this, by not taking the base ring, if the object is already a ring.

kliem commented 4 years ago

Commit: a272cfd

kliem commented 4 years ago

New commits:

a272cfdfix coercion of polyhedra with number fields
kliem commented 4 years ago

Branch: public/28770

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from a272cfd to a0068cc

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

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

a0068ccadded a doctest for the original bug
fchapoton commented 4 years ago
comment:4

reviewer full name is missing

kliem commented 4 years ago

Reviewer: Léo Brunswic

vbraun commented 4 years ago

Changed branch from public/28770 to a0068cc