#17569 closed enhancement (fixed)
Allow creating finite fields without a variable name
Reported by: | pbruin | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-7.1 |
Component: | finite rings | Keywords: | |
Cc: | ncohen, vdelecroix, jpflori, dimpase, bruno, jdemeyer, vbraun | Merged in: | |
Authors: | David Roe | Reviewers: | Volker Braun |
Report Upstream: | N/A | Work issues: | |
Branch: | 2a92ef3 (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description
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
Change History (23)
comment:1 Changed 6 years ago by
- Branch set to u/roed/allow_creating_finite_fields_without_a_variable_name
comment:2 Changed 6 years ago by
- Commit set to a56ec7453875e5caeface30c5c7689402cf04fba
- Milestone changed from sage-6.5 to sage-7.0
- Status changed from new to needs_review
comment:3 Changed 6 years ago by
- Milestone changed from sage-7.0 to sage-7.1
- Status changed from needs_review to needs_work
Why did you remove the whole # The following is a temporary solution...
block? Do you want conway
and prefix
to continue to be supported?
- If no => deprecate it.
- 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')
comment:4 follow-up: ↓ 5 Changed 6 years ago by
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
comment:5 in reply to: ↑ 4 Changed 6 years ago by
Replying to ncohen:
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
comment:6 Changed 6 years ago by
- Commit changed from a56ec7453875e5caeface30c5c7689402cf04fba to 2fcbc08667d7048d1f9a2691c8e4e9a2eff66001
comment:7 Changed 6 years ago by
- Status changed from needs_work to needs_review
Okay, I've added some documentation about how the conway and prefix keywords are handled.
comment:8 Changed 6 years ago by
What's the point?
This keyword argument is now silently ignored.
Sounds like it should be deprecated...
comment:9 follow-ups: ↓ 10 ↓ 11 ↓ 12 Changed 6 years ago by
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?
comment:10 in reply to: ↑ 9 Changed 6 years ago by
Replying to roed:
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^3I 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).
comment:11 in reply to: ↑ 9 Changed 6 years ago by
Replying to roed:
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
comment:12 in reply to: ↑ 9 Changed 6 years ago by
Replying to roed:
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")
comment:13 Changed 6 years ago by
- Status changed from needs_review to needs_work
comment:14 Changed 6 years ago by
- Commit changed from 2fcbc08667d7048d1f9a2691c8e4e9a2eff66001 to 9d7c717c1afbb0d7a7abd1ad93c0c3d63043e6ad
Branch pushed to git repo; I updated commit sha1. New commits:
9d7c717 | Deprecating conway keyword argument to finite field constructor
|
comment:15 Changed 6 years ago by
Conflict due to #19941. Thanks Nathann.
comment:16 Changed 6 years ago by
Oh true, the rebase. Right. Give me a second.
comment:17 Changed 6 years ago by
Helloooooo !
As expected, it was a nightmare :-P
You will find the rebased branch at u/ncohen/17569
.
Nathann
comment:18 Changed 6 years ago by
- Branch changed from u/roed/allow_creating_finite_fields_without_a_variable_name to u/ncohen/17569
- Commit changed from 9d7c717c1afbb0d7a7abd1ad93c0c3d63043e6ad to 69ecf50c5fc1a7ad794ad7e3e40f0493314b5d04
- Status changed from needs_work to needs_review
Cool. Thanks for the rebase.
comment:19 Changed 6 years ago by
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
comment:20 Changed 6 years ago by
- Branch changed from u/ncohen/17569 to u/roed/17569
comment:21 Changed 6 years ago by
- Commit changed from 69ecf50c5fc1a7ad794ad7e3e40f0493314b5d04 to 2a92ef3568d32310b1b63d99dc54c313f58d671e
- Reviewers set to Volker Braun
- Status changed from needs_review to positive_review
New commits:
2a92ef3 | Fixing doctest errors from conway kwd deprecation
|
comment:22 Changed 6 years ago by
- Branch changed from u/roed/17569 to 2a92ef3568d32310b1b63d99dc54c313f58d671e
- Resolution set to fixed
- Status changed from positive_review to closed
comment:23 Changed 16 months ago by
- Commit 2a92ef3568d32310b1b63d99dc54c313f58d671e deleted
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.
New commits:
Enabling GF(p^n) with no variable name given