Opened 9 years ago
Closed 9 years ago
#14833 closed enhancement (fixed)
Make choosing irreducible polynomials independent of finite field implementations
Reported by: | Peter Bruin | Owned by: | Clément Pernet |
---|---|---|---|
Priority: | major | Milestone: | sage-5.12 |
Component: | finite rings | Keywords: | polynomials |
Cc: | Xavier Caruso | Merged in: | sage-5.12.beta2 |
Authors: | Peter Bruin | Reviewers: | Jean-Pierre Flori |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #14832 | Stopgaps: |
Description (last modified by )
This patch makes the FiniteField
constructor call the implementation-independent code for choosing irreducible polynomials from #14832.
With this patch, the generic constructor always passes an actual polynomial (not a string specifiying an algorithm to construct one) to the finite field implementation class. For backwards compatibility, the existing classes still recognise a string modulus
. Checking if the finite field is a Conway polynomial is now done by an implementation-independent function.
This is a dependency of #12142.
Attachments (2)
Change History (14)
comment:1 Changed 9 years ago by
Description: | modified (diff) |
---|---|
Status: | new → needs_review |
comment:2 Changed 9 years ago by
Cc: | Xavier Caruso added |
---|
comment:3 follow-up: 4 Changed 9 years ago by
comment:4 Changed 9 years ago by
Replying to jpflori:
It may be better to use a function decorator to do the caching of the is_conway method, i.e. use @cached_method.
OK, I'll fix this as soon as I have time, unless you or someone else does it first.
And is there any point to prevent setting the default return value for finite fields explicitely constructed with conway polynomials? (i.e. removing what would be the equivalent of "self._is_conway = True" after the "modulus == "conway"" tests? to set the default value of the cached_method you can use "self.is_conway.set_cache(True)".)
That might be slightly more elegant, but the test for modulus == 'conway'
is now in PolynomialRing.irreducible_element
, which is not directly related to finite fields, so we would need an additional check in the finite field constructor. I'm not sure if it is worth it: checking whether we have a Conway polynomial is (hopefully) very fast (since the database was loaded when we looked up the Conway polynomial), and is_conway()
is currently only used for Sage <-> Gap conversion.
Changed 9 years ago by
Attachment: | trac_14833-FiniteField_polynomial_choice.patch added |
---|
based on 5.11.beta3 + #14832, use @cached_method
comment:5 Changed 9 years ago by
Description: | modified (diff) |
---|
comment:6 Changed 9 years ago by
Reviewers: | → Jean-Pierre Flori |
---|---|
Status: | needs_review → positive_review |
Ok, looks ok for me.
comment:9 Changed 9 years ago by
Maybe:
Apply: trac_14833-FiniteField_polynomial_choice.patch
That should prevent the Finite Field thing being interpreted by the trac syntax.
Changed 9 years ago by
Attachment: | trac_14833_finite_fields_polynomial_choice.patch added |
---|
empty patch as apéritif for patchbot
comment:10 Changed 9 years ago by
Now it thinks it is a digestif, but anyway...
(For the record, that file contained the first version of the patch; when I uploaded a revised version with a different name, the patchbot couldn't be made to eat only the new one.)
comment:12 Changed 9 years ago by
Merged in: | → sage-5.12.beta2 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
It may be better to use a function decorator to do the caching of the is_conway method, i.e. use @cached_method.
And is there any point to prevent setting the default return value for finite fields explicitely constructed with conway polynomials? (i.e. removing what would be the equivalent of "self._is_conway = True" after the "modulus == "conway"" tests? to set the default value of the cached_method you can use "self.is_conway.set_cache(True)".)