Changes between Version 18 and Version 21 of Ticket #13215


Ignore:
Timestamp:
10/22/12 14:42:22 (9 years ago)
Author:
caruso
Comment:

Thanks a lot for all your comments!

Replying to burcin:

  • short representations for morphisms
  • hash functions for morphisms (#9016)
  • q_jordan
  • modular exponentiation of polynomial_element

Ok, I've done it (see tickets #13640, #13641 #13642 and old ticket #13214 for "hash functions for morphisms").

After applying the patches on this ticket and its dependencies, I get the following error:

sage: R.<t> = ZZ[]
sage: sigma = R.hom([t+1])
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
sage: x*t
---------------------------------------------------------------------------
NotImplementedError                       Traceback (most recent call last)
...
sage: %debug
> /home/burcin/sage/sage-5.2/skewpolynomial_element.pyx(347)sage.rings.polynomial.skewpolynomial_element.SkewPolynomial._list_c (sage/rings/polynomial/skewpolynomial_element.c:4716)()

It's now fixed. (I haven't really understood what was the problem. It worked well on sage 5.1.0. I just removed some inline somewhere.)

  • 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 /.

Done.

  • 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:

It's not possible to remove this limitation this way because of the preparser (you see in the following example that the variable 'x' doesn't appear in the first instruction on the last line).

sage: k.<t> = GF(5^3)
sage: Frob = k.frobenius_endomorphism()
sage: preparse(S.<x> = k[Frob])
sage: preparse("S.<x> = k[Frob]")
'S = k[Frob]; (x,) = S._first_ngens(1)'
  • I suggest using skew_polynomial in the file names instead of skewpolynomial.

Done.

  • 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>?

Sometimes, Left is the default (for lcm). I agree that we can just have a boolean argument but, when I wrote this code, I thought that it was more elegant to do this way. Anymay, I can change if you insist :-).

  • There are many functions missing a docstring or tests.

I've added some documentation and doctests (but I'm still far from a 100% coverage).

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #13215

    • Property Dependencies changed from #13214, #13303 to #13214, #13303, #13640, #13641, #13642
    • Property Reviewers changed from to Burcin Erocal
  • Ticket #13215 – Description

    v18 v21  
    881. a more complete implementation of skew polynomials over finite fields (including factoring)
    99
    10 NB: This ticket depends on tickets #13214 (which implements Frobenius endomorphisms over finite fields) and #13303 (which fixes a bug in quotient_polynomial_ring_element.pyx). For convenience, I reattach the corresponding patches here (you need to apply these two patches first).
     10NB: This ticket depends on tickets #13214 (Frobenius endomorphisms over finite fields), #13303 (which fixes a bug in quotient_polynomial_ring_element.pyx), #13640 (some q-numbers), #13641 (short representation for morphisms), #13642 (fast modular exponentiation, only for speed). You need to apply the corresponding patches first.
    1111
    1212Apply: attachment:trac_13215_skew_polynomials.patch