Skip to content

Commit

Permalink
sagemathgh-38441: number_field_elements_from_algebraics: Fix Cyclotom…
Browse files Browse the repository at this point in the history
…icField embedding when embedding=False

    
Fixes sagemath#38440.

Uses `.coerce_embedding()` to determine the existence of the embedding
and modify the field accordingly, instead of doing what the code does
currently (assume the field is unembedded by default).

Note that there's a behavior change --- now when `embedding=False` it
will return a `NumberField` instead of `CyclotomicField` (even though it
is technically possible to create a `CyclotomicField(5,
embedding=None)`). I don't think this matters.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
- [x] I have updated the documentation and checked the documentation
preview. (only part that change is https://doc-pr-38441--sagemath.netlif
y.app/html/en/reference/number_fields/sage/rings/qqbar.html#sage.rings.q
qbar.number_field_elements_from_algebraics )
    
URL: sagemath#38441
Reported by: user202729
Reviewer(s): roed314, Travis Scrimshaw, user202729
  • Loading branch information
Release Manager committed Nov 13, 2024
2 parents 51e9058 + 6c56c15 commit d4a056c
Showing 1 changed file with 84 additions and 14 deletions.
98 changes: 84 additions & 14 deletions src/sage/rings/qqbar.py
Original file line number Diff line number Diff line change
Expand Up @@ -2556,7 +2556,8 @@ def number_field_elements_from_algebraics(numbers, minimal=False,
- ``same_field`` -- boolean (default: ``False``); see below
- ``embedded`` -- boolean (default: ``False``); whether to make the
NumberField embedded
NumberField embedded, note that this has no effect when the
resulting field is ``QQ`` because there is only one ``QQ`` instance
- ``name`` -- string (default: ``'a'``); name of the primitive element
Expand Down Expand Up @@ -2827,6 +2828,69 @@ def number_field_elements_from_algebraics(numbers, minimal=False,
To: Algebraic Real Field
Defn: 1 |--> 1)
Test ``embedded`` for quadratic and cyclotomic fields::
sage: v = number_field_elements_from_algebraics([QQbar((-1)^(2/3))], embedded=False, minimal=True); v
(Number Field in zeta6 with defining polynomial x^2 - x + 1,
[zeta6 - 1],
Ring morphism:
From: Number Field in zeta6 with defining polynomial x^2 - x + 1
To: Algebraic Field
Defn: zeta6 |--> 0.500000000000000? + 0.866025403784439?*I)
sage: v[0].coerce_embedding()
sage: v = number_field_elements_from_algebraics([QQbar((-1)^(2/3))], embedded=True, minimal=True); v
(Cyclotomic Field of order 6 and degree 2,
[zeta6 - 1],
Ring morphism:
From: Cyclotomic Field of order 6 and degree 2
To: Algebraic Field
Defn: zeta6 |--> 0.500000000000000? + 0.866025403784439?*I)
sage: v[0].coerce_embedding()
Generic morphism:
From: Cyclotomic Field of order 6 and degree 2
To: Complex Lazy Field
Defn: zeta6 -> 0.500000000000000? + 0.866025403784439?*I
sage: v = number_field_elements_from_algebraics([QQbar((-1)^(1/2))], embedded=False, minimal=True); v
(Number Field in I with defining polynomial x^2 + 1,
[I],
Ring morphism:
From: Number Field in I with defining polynomial x^2 + 1
To: Algebraic Field
Defn: I |--> 1*I)
sage: v[0].coerce_embedding()
sage: v = number_field_elements_from_algebraics([QQbar((-1)^(1/2))], embedded=True, minimal=True); v
(Number Field in I with defining polynomial x^2 + 1 with I = 1*I,
[I],
Ring morphism:
From: Number Field in I with defining polynomial x^2 + 1 with I = 1*I
To: Algebraic Field
Defn: I |--> 1*I)
sage: v[0].coerce_embedding()
Generic morphism:
From: Number Field in I with defining polynomial x^2 + 1 with I = 1*I
To: Complex Lazy Field
Defn: I -> 1*I
sage: v = number_field_elements_from_algebraics([QQbar((-1)^(1/5))], embedded=False, minimal=True); v
(Number Field in zeta10 with defining polynomial x^4 - x^3 + x^2 - x + 1,
[zeta10],
Ring morphism:
From: Number Field in zeta10 with defining polynomial x^4 - x^3 + x^2 - x + 1
To: Algebraic Field
Defn: zeta10 |--> 0.8090169943749474? + 0.5877852522924731?*I)
sage: v[0].coerce_embedding()
sage: v = number_field_elements_from_algebraics([QQbar((-1)^(1/5))], embedded=True, minimal=True); v
(Cyclotomic Field of order 10 and degree 4,
[zeta10],
Ring morphism:
From: Cyclotomic Field of order 10 and degree 4
To: Algebraic Field
Defn: zeta10 |--> 0.8090169943749474? + 0.5877852522924731?*I)
sage: v[0].coerce_embedding()
Generic morphism:
From: Cyclotomic Field of order 10 and degree 4
To: Complex Lazy Field
Defn: zeta10 -> 0.809016994374948? + 0.587785252292473?*I
Tests more complicated combinations::
sage: # needs sage.libs.gap sage.symbolic
Expand All @@ -2847,10 +2911,10 @@ def number_field_elements_from_algebraics(numbers, minimal=False,
sage: AA((-1)^(2/3))
1
sage: number_field_elements_from_algebraics([(-1)^(2/3)])
(Cyclotomic Field of order 6 and degree 2,
(Number Field in zeta6 with defining polynomial x^2 - x + 1,
[zeta6 - 1],
Ring morphism:
From: Cyclotomic Field of order 6 and degree 2
From: Number Field in zeta6 with defining polynomial x^2 - x + 1
To: Algebraic Field
Defn: zeta6 |--> 0.500000000000000? + 0.866025403784439?*I)
"""
Expand Down Expand Up @@ -2905,17 +2969,23 @@ def mk_algebraic(x):
exact_generator = gen.root_as_algebraic()
hom = fld.hom([exact_generator])

if fld is not QQ and embedded:
# creates the embedded field
embedded_field = NumberField(fld.defining_polynomial(), fld.variable_name(), embedding=exact_generator)

# embeds the numbers
inter_hom = fld.hom([embedded_field.gen(0)])
nums = [inter_hom(n) for n in nums]

# get the field and homomorphism
hom = embedded_field.hom([exact_generator])
fld = embedded_field
if fld is not QQ:
# ignore the embedded parameter for QQ
# cyclotomic fields are embedded by default
# number fields are not embedded by default
# if the default embedding is different from what is expected then modify the field
if embedded != (fld.coerce_embedding() is not None):
# creates the modified field
modified_field = NumberField(fld.defining_polynomial(), fld.variable_name(),
embedding=exact_generator if embedded else None)

# embeds the numbers
inter_hom = fld.hom([modified_field.gen(0)])
nums = [inter_hom(n) for n in nums]

# get the field and homomorphism
hom = modified_field.hom([exact_generator])
fld = modified_field

if single_number:
nums = nums[0]
Expand Down

0 comments on commit d4a056c

Please sign in to comment.