Automatic variable names for finite fields are not normalized

[xcaruso]

There is a problem with the normalization of names of variables which are given automatically for finite fields.

sage: K = GF(4)
sage: K._factory_data
(<class 'sage.rings.finite_rings.finite_field_constructor.FiniteFieldFactory'>,
 (7, 5, 'beta6'),
 (4, 'z2', x^2 + x + 1, 'givaro', "{'prefix': 'z'}", 2, 2, True), {'prefix': 'z'})
     ^^^^

Here the name of the variable(s) is not normalized and should be ('z2',). Compare with:

sage: L = GF(4, name='z2')
sage: L._factory_data
(<class 'sage.rings.finite_rings.finite_field_constructor.FiniteFieldFactory'>,
 (7, 5, 'beta6'),
 (4, ('z2',), x^2 + x + 1, 'givaro', '{}', 2, 2, True),
 {})

By the way, should K == L. Currently, it is false.

CC: @jpflori @sagetrac-erousseau

Component: finite rings

Keywords: name normalization, finite fields, sd86.5

Author: Xavier Caruso

Branch/Commit: u/caruso/normalize_name_conway @ c7870d3

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

[xcaruso]

Branch: u/caruso/normalize_name_conway

[xcaruso]

Commit: 82bf26c

[xcaruso]

New commits:

82bf26c

Normalizing names of automatic variables of finite fields

[xcaruso]

Description changed:

--- 
+++ 
@@ -20,4 +20,4 @@
  {})

-By the way, should K == L. Currently, it is false. +By the way, should K == L? Currently, it is false.

[saraedum]

comment:4

I think it's our policy that changes should come with a doctest. I think K == L and K._factory_data would be nice doctests.

[saraedum]

Description changed:

--- 
+++ 
@@ -20,4 +20,4 @@
  {})

-By the way, should K == L? Currently, it is false. +By the way, should K == L. Currently, it is false.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 82bf26c to 25f1b16

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

25f1b16

Default prefix 'z' for all finite field + inheritance by extensions

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

b2dba64

Prefix for prime fields as well (otherwise coercion is broken)

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 25f1b16 to b2dba64

[pjbruin]

comment:7

I have two comments:

1) The following should definitely NOT return True:

sage: GF(17^180) == GF(17^180, 'z180')
False

The first syntax (which was enabled "on popular request", but is too seductive in my opinion) creates the finite field as a subfield of an algebraic closure, which is expensive because it has to create a system of compatible (pseudo-Conway) polynomials. Compare the following timings:

sage: %time GF(17^180, 'z180')
CPU times: user 64 ms, sys: 8 ms, total: 72 ms
Wall time: 71 ms
Finite Field in a of size 17^180
sage: %time GF(17^180)
CPU times: user 8.62 s, sys: 36 ms, total: 8.66 s
Wall time: 8.66 s
Finite Field in z180 of size 17^180

2) The prefix parameter was introduced to store the name used for (subfields of) an algebraic closure. I don't think it should be added to every finite field regardless of whether it is embedded in an algebraic closure.

I propose to limit this ticket to just normalising the variable (with a doctest showing the correct factory data), nothing else.

[eb007249-102c-4be0-927e-dc76b4746a2c]

comment:9

Hello,

Should we go back to commit 82bf26c with a doctest testing ._factory_data then ?

Edouard

[saraedum]

comment:11

erousseau, if you have a proposal for a fix, I think that you can just go ahead and push it to this ticket. The old implementation is not lost, so we could always go back if we wanted to.

[saraedum]

Changed keywords from name normalization, finite fields to name normalization, finite fields, sd86.5

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

c708c16	Revert "Prefix for prime fields as well (otherwise coercion is broken)"
298ca57	Revert "Default prefix 'z' for all finite field + inheritance by extensions"
adbed68	Add doctest
c7870d3	Merge branch 'develop' into normalize_name_conway

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from b2dba64 to c7870d3

[xcaruso]

comment:14

Fine, I agree for limiting this ticket to name normalization.

[roed314]

comment:15

(7, 5, 'beta6') should be replaced by ....