#13215: Skew polynomials
------------------------------------+---------------------------------------
Reporter: caruso | Owner: tbd
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-5.4
Component: algebra | Resolution:
Keywords: skew polynomials | Work issues:
Report Upstream: N/A | Reviewers: Burcin Erocal
Authors: Xavier Caruso | Merged in:
Dependencies: #13214, #13303 | Stopgaps:
------------------------------------+---------------------------------------
Changes (by burcin):
* status: needs_review => needs_work
* reviewer: => Burcin Erocal
Comment:
Replying to [comment:16 burcin]:
> Replying to [comment:15 caruso]:
> > Ok, I will split my patch in several components. (Is two enough or are
you expecting more than that?).
>
> I don't know yet. It depends on how many functionally independent parts
there are. For instance, the short representations for morphisms can be an
independent piece that is trivial to review. The hash functions for
morphisms would also be a separate bug fix. Actually, I attempted to fix
that a long time ago at #9016.
From a cursory reading of attachment:trac_13215_skew_polynomials.patch, I
see these separate parts that should have a ticket of its own:
- short representations for morphisms
- hash functions for morphisms (#9016)
- q_jordan
- modular exponentiation of polynomial_element
A few comments (not a full review):
- the operator `/` always constructs the quotient domain in Sage, even if
exact division is possible. Since in this case we need a generic Ore
localization for `/`, I suggest you only implement exact division with
`//` and raise an error on `/`.
{{{
+ sage: d = a * b
+ sage: d/b == a # Right division
+ True
+
+ sage: c/b
+ Traceback (most recent call last):
+ ...
+ ValueError: the division is not exact
}}}
- For the constructor, why don't you check if the argument of the
`__getitem__` method is a `Morphism` instead of this:
{{{
from sage.categories.morphism import Morphism
if isinstance(x,tuple) and len(x) == 2 and
isinstance(x[1],Morphism):
}}}
This should remove the limitation of having to specify the variable name
on the right hand side as documented here:
{{{
+One can also use a shorter syntax::
+
+ sage: S.<x> = R['x',sigma]; S
+ Skew Polynomial Ring in x over Univariate Polynomial Ring in t over
Integer Ring twisted by t |--> t + 1
+
+Be careful, with the latter syntax, one cannot omit the name of the
+variable neither in LHS nor in RHS. If we omit it in LHS, the variable
+is not created::
+
+ sage: Sy = R['y',sigma]; Sy
+ Skew Polynomial Ring in y over Univariate Polynomial Ring in t over
Integer Ring twisted by t |--> t + 1
+ sage: y.parent()
+ Traceback (most recent call last):
+ ...
+ NameError: name 'y' is not defined
+
+If we omit it in RHS, sage tries to create a polynomial ring and fails::
+
+ sage: Sz.<z> = R[sigma]
+ Traceback (most recent call last):
+ ...
+ ValueError: variable names must be alphanumeric, but one is 'Ring
endomorphism of Univariate Polynomial Ring in t over Integer Ring
+ Defn: t |--> t + 1' which is not.
}}}
- There is a class `SkewPolynomialRing_finite_field` defined in
`sage/rings/polynomial/skewpolynomial_ring.py`. There is also a file
`sage/rings/polynomial/skewpolynomial_finite_field.pyx`. It seems that the
class defined in the latter does not use the latter:
{{{
cdef class SkewPolynomial_finite_field_dense
(SkewPolynomial_generic_dense):
}}}
- I suggest using `skew_polynomial` in the file names instead of
`skewpolynomial`.
- Are the `Left` and `Right` objects defined in `sage/structure/side.py`
really necessary? If `Right` is the default, can't you just have a keyword
argument `left=<bool>`?
- There are many functions missing a docstring or tests. Here is partial
coverage output:
{{{
$ ./sage -coverage devel/sage/sage/rings/polynomial/skew*
----------------------------------------------------------------------
devel/sage/sage/rings/polynomial/skewpolynomial_element.pyx
ERROR: Please add a `TestSuite(s).run()` doctest.
SCORE devel/sage/sage/rings/polynomial/skewpolynomial_element.pyx: 78% (58
of 74)
Missing documentation:
* __hash__(self):
* __richcmp__(left, right, int op):
* __nonzero__(self):
* _is_atomic(self):
* __lshift__(self, k):
* __rshift__(self, k):
* is_gen(self):
* make_generic_skewpolynomial(parent, coeffs):
* Element _call_(self, x):
* __init__(self, domain, codomain):
* Element _call_(self, x):
* Element _call_with_args(self, x, args=(), kwds={}):
* section(self):
Missing doctests:
* SkewPolynomial _new_constant_poly(self,RingElement a,Parent
P,char check=0):
* is_unit(self):
* __copy__(self):
Possibly wrong (function name doesn't occur in doctests):
* ModuleElement _add_(self, ModuleElement right):
* ModuleElement _sub_(self, ModuleElement right):
* ModuleElement _neg_(self):
* ModuleElement _lmul_(self, RingElement right):
* ModuleElement _rmul_(self, RingElement left):
* RingElement _mul_(self, RingElement right):
* RingElement _div_(self, RingElement right):
----------------------------------------------------------------------
----------------------------------------------------------------------
devel/sage/sage/rings/polynomial/skewpolynomial_finite_field.pyx
ERROR: Please add a `TestSuite(s).run()` doctest.
SCORE devel/sage/sage/rings/polynomial/skewpolynomial_finite_field.pyx:
82% (29 of 35)
Missing documentation:
* __init__(self, parent, x=None, int check=1, is_gen=False, int
construct=0, **kwds):
* reduced_norm_factor(self):
* _factorizations_rec(self):
Missing doctests:
* _irreducible_divisors(self,side):
* __init__(self,parent,cutoff=0):
* __repr__(self):
Possibly wrong (function name doesn't occur in doctests):
* RingElement _mul_(self, RingElement right):
----------------------------------------------------------------------
----------------------------------------------------------------------
devel/sage/sage/rings/polynomial/skewpolynomial_ring_constructor.py
SCORE devel/sage/sage/rings/polynomial/skewpolynomial_ring_constructor.py:
100% (1 of 1)
----------------------------------------------------------------------
----------------------------------------------------------------------
devel/sage/sage/rings/polynomial/skewpolynomial_ring.py
SCORE devel/sage/sage/rings/polynomial/skewpolynomial_ring.py: 42% (17 of
40)
Missing documentation:
* _call_ (self, x):
* __init__(self,domain,codomain,embed,order):
* _repr_(self):
* _call_(self,x):
* section(self):
* __init__ (self, skew_ring, names=None, sparse=False,
element_class=None):
* __classcall__(cls, base_ring, map, name=None, sparse=False,
element_class=None):
* __init__(self, base_ring, map, name, sparse, element_class):
* __reduce__(self):
* _element_constructor_(self, x=None, check=True, is_gen = False,
construct=False, **kwds):
* _coerce_map_from_(self, P):
* __cmp__(left, right):
* _repr_(self):
* _latex_(self):
* gens_dict(self):
* __classcall__(cls, base_ring, map, name=None, sparse=False,
element_class=None):
* __init__(self, base_ring, map, name, sparse, element_class):
Missing doctests:
* _repr_ (self):
* _latex_ (self):
* parameter(self):
* is_sparse(self):
* _new_retraction_map(self,alea=None):
* _retraction(self,x,newmap=False,alea=None):
----------------------------------------------------------------------
}}}
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/13215#comment:19>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
