Opened 5 years ago
Closed 5 years ago
#16509 closed defect (fixed)
bug in .as_finite_field_element() when minimal=True
Reported by: | vdelecroix | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.3 |
Component: | finite rings | Keywords: | bug |
Cc: | pbruin, mmarco, defeo | Merged in: | |
Authors: | Peter Bruin | Reviewers: | Vincent Delecroix |
Report Upstream: | N/A | Work issues: | |
Branch: | 49ce558 (Commits) | Commit: | 49ce5587517a69cc32215cd4763ab10de55a76b2 |
Dependencies: | Stopgaps: |
Description (last modified by )
AlgebraicClosureFiniteField.as_finite_field_element(minimal=True)
fails when it needs to convert between two non-trivial finite subfields:
sage: K = GF(5).algebraic_closure() sage: z=K.gen(5) - K.gen(5) + K.gen(2) sage: z.as_finite_field_element(minimal=True) Traceback (most recent call last): ... TypeError: unable to coerce
Note that it works fine when minimal=False
(which is the default).
Change History (13)
comment:1 Changed 5 years ago by
comment:2 follow-up: ↓ 4 Changed 5 years ago by
I would prefer that the following works...
sage: K = GF(5).algebraic_closure() sage: K3 = K.subfield(3)[0] sage: K6 = K.subfield(6)[0] sage: K3.gen() z3 sage: K6(K3.gen()) Traceback (most recent call last): ... TypeError: unable to coerce from a finite field other than the prime subfield
comment:3 Changed 5 years ago by
- Branch set to u/pbruin/16509-as_finite_field_element
- Commit set to 5572837f991d5ed6a1674e009de8c38e0c3bbc7c
- Description modified (diff)
- Status changed from new to needs_review
comment:4 in reply to: ↑ 2 Changed 5 years ago by
Replying to vdelecroix:
I would prefer that the following works...
sage: K = GF(5).algebraic_closure() sage: K3 = K.subfield(3)[0] sage: K6 = K.subfield(6)[0] # not used sage: K3.gen() z3 sage: K6(K3.gen()) Traceback (most recent call last): ... TypeError: unable to coerce from a finite field other than the prime subfield
In my opinion, from a mathematical perspective this should not be expected to work. The first component of the tuple returned by subfield()
method is just the abstract field, and there is no canonical map between non-trivial "stand-alone" finite fields. Either stay inside the algebraic closure, or otherwise use the inclusion()
method to obtain a map that you can use outside of it:
sage: K = GF(5).algebraic_closure() sage: K3 = K.subfield(3)[0] sage: f = K.inclusion(3, 6) sage: f(K3.gen()) 4*z6^5 + z6^4 + 4*z6^3 + 4*z6^2 + 3*z6 + 3 sage: f.codomain() is K.subfield(6)[0] True
comment:5 follow-up: ↓ 7 Changed 5 years ago by
I see. Do you mind if I add this example to the branch?
comment:6 Changed 5 years ago by
Hi Peter,
And, one more remark, with QQbar when such method is called the internal representation is changed:
sage: a = QQbar(2^(1/4)) sage: b = a**2 sage: b.as_number_field_element() (Number Field in a with defining polynomial y^4 - 2, a^2, Ring morphism: From: Number Field in a with defining polynomial y^4 - 2 To: Algebraic Real Field Defn: a |--> 1.189207115002722?) sage: b.as_number_field_element(minimal=True) (Number Field in a with defining polynomial y^2 - 2, a, Ring morphism: From: Number Field in a with defining polynomial y^2 - 2 To: Algebraic Real Field Defn: a |--> 1.414213562373095?) sage: b.as_number_field_element(minimal=False) (Number Field in a with defining polynomial y^2 - 2, a, Ring morphism: From: Number Field in a with defining polynomial y^2 - 2 To: Algebraic Real Field Defn: a |--> 1.414213562373095?)
Do we want the same here?
Vincent
comment:7 in reply to: ↑ 5 ; follow-up: ↓ 8 Changed 5 years ago by
Replying to vdelecroix:
I see. Do you mind if I add this example to the branch?
Why not, please go ahead.
I don't think we should change the internal representation when the user calls as_number_field_element()
. Currently, two elements that compare equal can have a different hash (which is a bad thing); changing the internal representation of an element would therefore mean that its hash can change, which is even worse.
QQbar is careful enough to ensure that equal elements with different representations have the same hash; it would be nice if we could somehow do that too.
comment:8 in reply to: ↑ 7 ; follow-up: ↓ 9 Changed 5 years ago by
Replying to pbruin:
Replying to vdelecroix: I don't think we should change the internal representation when the user calls
as_number_field_element()
. Currently, two elements that compare equal can have a different hash (which is a bad thing); changing the internal representation of an element would therefore mean that its hash can change, which is even worse.
right
QQbar is careful enough to ensure that equal elements with different representations have the same hash; it would be nice if we could somehow do that too.
I know but they "cheat" using the fact that you have a natural embedding to the complex field. Do you have an idea to get a (fast) coherent hash for algebraic closure elements?
comment:9 in reply to: ↑ 8 Changed 5 years ago by
Replying to vdelecroix:
Do you have an idea to get a (fast) coherent hash for algebraic closure elements?
The only thing I can think of is to take the hash of the minimal polynomial, which unfortunately (1) is not very fast and (2) gives the same result for all conjugates of an element.
[Edit: or take the hash after coercing to the minimal subfield containing the element, which solves (2) but not (1).]
comment:10 Changed 5 years ago by
I think that, the same way that there are "number fields with fixed embeddings on QQbar", there should be "finite filed with fixed embeddings on the algebraic closure". That way
sage: K = GF(5).algebraic_closure() sage: K3 = K.subfield(3)[0] sage: K6 = K.subfield(6)[0] # not used sage: K3.gen() z3 sage: K6(K3.gen()) Traceback (most recent call last): ... TypeError: unable to coerce from a finite field other than the prime subfield
Could be made to work and also make mathematical sense.
comment:11 Changed 5 years ago by
Hi Peter,
I added the doc at public/16509
and at the same time correct a typo. If you are fine with it, set to positive review.
Vincent
comment:12 Changed 5 years ago by
- Branch changed from u/pbruin/16509-as_finite_field_element to public/16509
- Commit changed from 5572837f991d5ed6a1674e009de8c38e0c3bbc7c to 49ce5587517a69cc32215cd4763ab10de55a76b2
- Reviewers set to Vincent Delecroix
- Status changed from needs_review to positive_review
comment:13 Changed 5 years ago by
- Branch changed from public/16509 to 49ce5587517a69cc32215cd4763ab10de55a76b2
- Resolution set to fixed
- Status changed from positive_review to closed
The finite subfields should not even have to talk to each other; it is
as_finite_field_element()
that should take care of the conversion, in a similar way aschange_level()
. I made a small patch which I am now testing.