Allow creating finite fields without a variable name

[pjbruin]

It should be possible to use FiniteField(p^n) (or FiniteField(p, n), see #17568) without a name argument to create the subfield of FiniteField(p).algebraic_closure() of cardinality p^n. This would mean

GF(p, n) = GF(p^n) = GF(p).algebraic_closure().subfield(n)[0]

Discussion on sage-devel: https://groups.google.com/forum/#!topic/sage-devel/LlstHp950uo

CC: @nathanncohen @videlec @jpflori @dimpase @bgrenet @jdemeyer @vbraun

Component: finite rings

Author: David Roe

Branch: 2a92ef3

Reviewer: Volker Braun

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

[roed314]

Branch: u/roed/allow_creating_finite_fields_without_a_variable_name

[roed314]

Author: David Roe

[roed314]

Commit: a56ec74

[roed314]

New commits:

a56ec74

Enabling GF(p^n) with no variable name given

[jdemeyer]

comment:3

Why did you remove the whole # The following is a temporary solution... block? Do you want conway and prefix to continue to be supported?

1. If no => deprecate it.
2. If yes => document it properly (i.e. move the comment you just removed to the actual docstring) and add some doctests like

sage: GF(3^10, conway=True, prefix='z') is GF(3^10)
sage: GF(3^10, conway=True, prefix='foo')

[nathanncohen]

comment:4

Hello !

I am affraid that I am not sufficiently skilled to review this branch, but I added a small commit at public/17569 which fixes a couple of typos and adds a link.

Nathann

[roed314]

comment:5

Replying to @nathanncohen:

Hello !

I am affraid that I am not sufficiently skilled to review this branch, but I added a small commit at public/17569 which fixes a couple of typos and adds a link.

Great. I have to go teach now, but I'll take a look in the next day or so.

Nathann

[sagetrac-git]

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

d00e732	trac #17569: Typos
2fcbc08	Addressing Jeroen's comments about the conway and prefix keywords for 17569.

[sagetrac-git]

Changed commit from a56ec74 to 2fcbc08

[roed314]

comment:7

Okay, I've added some documentation about how the conway and prefix keywords are handled.

[jdemeyer]

comment:8

What's the point?

This keyword argument is now silently ignored.

Sounds like it should be deprecated...

[roed314]

comment:9

Currently, you can feed random keyword arguments to most (all?) of the finite field constructors and they'll have no effect.

sage: GF(27,'a',blah=True)
Finite Field in a of size 3^3

I don't see a problem in having conway behave in the same manner. Why raise a deprecation warning when the code can just keep working without any change?

[dimpase]

comment:10

Replying to @roed314:

Currently, you can feed random keyword arguments to most (all?) of the finite field constructors and they'll have no effect.

sage: GF(27,'a',blah=True)
Finite Field in a of size 3^3

I don't see a problem in having conway behave in the same manner. Why raise a deprecation warning when the code can just keep working without any change?

You are effecting a change in the behaviour of the code. E.g.
sage: GF(27,'a',conway=True) currently throws an exception, and this is will no longer be the case. (Perhaps one can come up with a more interesting example, but that's beside the point).

[nathanncohen]

comment:11

Replying to @roed314:

Currently, you can feed random keyword arguments to most (all?) of the finite field constructors and they'll have no effect.

These useless-but-accepted arguments are a plague. There was a wealth of them in many 'plot' functions at a time: you tried to give a color to something, the argument 'color="red"' was accepted but nothing happened.

The most common way in which they are a pain are typos:

sage: GF(16,'x',checkirreducible=True)
Finite Field in x of size 2^4

The side-effect of not having a '_' in check_irreducible is that... It is ignored. And of course no exception to tell you.

About having an effect, here is one (thanks to UniqueRepresentation, I expect)

sage: GF(27,'a') == GF(27,'a')
True
sage: GF(27,'a',whatever=158) == GF(27,'a')
False
sage: GF(27,'a')(1) + GF(27,'a')(1)
2
sage: GF(27,'a',whatever=158)(1) + GF(27,'a')(1)
TypeError: unsupported operand parent(s) for '+': 'Finite Field in a of size 3^3' and 'Finite Field in a of size 3^3'

Nathann

[jdemeyer]

comment:12

Replying to @roed314:

Currently, you can feed random keyword arguments to most (all?) of the finite field constructors and they'll have no effect.

That's certainly a serious bug, but outside the scope of this ticket.

Still, I think that conway needs to be deprecated because it used to do something. Users should be made aware that the conway argument is no longer needed. Something like this should work:

if 'conway' in kwds:
    del kwds['conway']
    from sage.misc.superseded import deprecation
    deprecation(17569, "the 'conway' argument is deprecated, pseudo-conway polynomials are now used by default if no variable name is given") 

[sagetrac-git]

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

9d7c717

Deprecating conway keyword argument to finite field constructor

[sagetrac-git]

Changed commit from 2fcbc08 to 9d7c717

[roed314]

comment:15

Conflict due to #19941. Thanks Nathann.

[nathanncohen]

comment:16

Oh true, the rebase. Right. Give me a second.

[nathanncohen]

comment:17

Helloooooo !

As expected, it was a nightmare :-P

You will find the rebased branch at u/ncohen/17569.

Nathann

[roed314]

comment:18

Cool. Thanks for the rebase.

[roed314]

Changed branch from u/roed/allow_creating_finite_fields_without_a_variable_name to u/ncohen/17569

[roed314]

Changed commit from 9d7c717 to 69ecf50

[nathanncohen]

comment:19

It was just the worst amount of 'bad'. Too bad to be 'easy', and not sufficiently bad to force me to learn how to merge two branches which work on a renamed file, in general.

Well. Next time :-P

[roed314]

Changed branch from u/ncohen/17569 to u/roed/17569

[vbraun]

Reviewer: Volker Braun

[vbraun]

Changed commit from 69ecf50 to 2a92ef3

[vbraun]

New commits:

2a92ef3

Fixing doctest errors from conway kwd deprecation

[vbraun]

Changed branch from u/roed/17569 to 2a92ef3

[slel]

comment:23

Using a pseudo-conway modulus by default means the function
can be very slow when using a random irreducible polynomial
as the modulus would make it very fast.

Changing the default is discussed at #31434 if anybody who
participated in this ticket wants to chime in.

[slel]

Changed commit from 2a92ef3 to none