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 pbruin)

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 pbruin

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 as change_level(). I made a small patch which I am now testing.

comment:2 follow-up: Changed 5 years ago by 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]
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 pbruin

  • Authors set to Peter Bruin
  • 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 pbruin

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: Changed 5 years ago by vdelecroix

I see. Do you mind if I add this example to the branch?

comment:6 Changed 5 years ago by vdelecroix

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: Changed 5 years ago by pbruin

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: Changed 5 years ago by vdelecroix

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 pbruin

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).]

Last edited 5 years ago by pbruin (previous) (diff)

comment:10 Changed 5 years ago by mmarco

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 vdelecroix

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 pbruin

  • 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 vbraun

  • Branch changed from public/16509 to 49ce5587517a69cc32215cd4763ab10de55a76b2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.