Opened 16 months ago

Last modified 8 weeks ago

#24526 needs_work defect

more user friendly finite field extensions

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-8.2
Component: number theory Keywords:
Cc: Merged in:
Authors: Vincent Delecroix Reviewers:
Report Upstream: N/A Work issues: merge conflict
Branch: u/vdelecroix/24526 (Commits) Commit: ba8454698b6ebc00afcebeefc5b586e2dfcff4be
Dependencies: Stopgaps:

Description (last modified by vdelecroix)

GF(p).extension(xxx) does not accept symbolic polynomials... and the error is cryptic

sage: Fp = GF(3)
sage: Fp2.<i> = Fp.extension(x^2+1)
Traceback (most recent call last):
UnboundLocalError: local variable 'E' referenced before assignment

We make it more user friendly by allowing the above in some duck typing fashion.

Change History (5)

comment:1 Changed 16 months ago by vdelecroix

  • Authors set to Vincent Delecroix
  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 16 months ago by vdelecroix

  • Branch set to u/vdelecroix/24526
  • Commit set to ba8454698b6ebc00afcebeefc5b586e2dfcff4be

New commits:

ba8454624526: more user friendly finite field extension

comment:3 Changed 16 months ago by mmezzarobba

It may be better to keep the branch dealing with tuples/lists for speed reasons... Perhaps it doesn't matter (I don't know how much it is used or what alternatives exist for constructing extensions without too much overhead), but your patch makes the call to extension() in GF(3).extension([1, 0, 1], 'a') 70% slower on my system.

comment:4 Changed 14 months ago by darij

Should this ducktyping really all be happening under the hood without notifying the user? I think a more readable error message would be better, or at least a warning. We don't want people to use SR for polynomials in their code, do we?

Also, is this

+                else:
+                    modulus = modulus.change_ring(self)

necessary? The GF constructor already does modulus = R(modulus).monic(), where R = PolynomialRing?(FiniteField?(p), 'x').

comment:5 Changed 8 weeks ago by dkrenn

  • Status changed from needs_review to needs_work
  • Work issues set to merge conflict

Does not apply anymore.

Note: See TracTickets for help on using tickets.