Opened 6 years ago
Closed 6 years ago
#18420 closed enhancement (fixed)
Uniformize truncated multiplication for polynomials
Reported by:  vdelecroix  Owned by:  

Priority:  major  Milestone:  sage6.7 
Component:  algebra  Keywords:  
Cc:  Merged in:  
Authors:  Vincent Delecroix  Reviewers:  Mario Pernici 
Report Upstream:  N/A  Work issues:  
Branch:  063516e (Commits, GitHub, GitLab)  Commit:  063516e84406367fdce86bdc7bf5e19cd9ec14ae 
Dependencies:  Stopgaps: 
Description (last modified by )
The operation _mul_trunc
on polynomials is currently only implemented in specialized classes with custom declaration. We define a global one in Polynomial with signature
class Polynomial: cpdef Polynomial _mul_trunc_(self, Polynomial right, long n)
(and deprecate the former _mul_trunc).
We also add specialized implentation for integer polynomials (relying on fmpz_poly_mullow
) and rational polynomials (relying on fmpq_poly_mullow
).
Such method would be really helpful for multiplication of power series.
Change History (17)
comment:1 Changed 6 years ago by
 Description modified (diff)
comment:2 Changed 6 years ago by
 Branch set to u/vdelecroix/18420
 Commit set to 446b952f32665697ee168f7fc61649f540a82e73
 Status changed from new to needs_review
comment:3 Changed 6 years ago by
 Commit changed from 446b952f32665697ee168f7fc61649f540a82e73 to 4e96421ff4be51ed7ff188281a0764f7c1a6ba7c
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
4e96421  Trac 18420: _mul_trunc_ for all polynomials

comment:4 Changed 6 years ago by
 Commit changed from 4e96421ff4be51ed7ff188281a0764f7c1a6ba7c to b73a8404319fc0f7542989c340376c43610bc1e0
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
b73a840  Trac 18420: _mul_trunc_ for all polynomials

comment:5 Changed 6 years ago by
rebased on 67.rc0
comment:6 Changed 6 years ago by
 Description modified (diff)
comment:7 Changed 6 years ago by
 Commit changed from b73a8404319fc0f7542989c340376c43610bc1e0 to 12d6361f7dc92fd0054da2b83bab60c59a3ad201
Branch pushed to git repo; I updated commit sha1. New commits:
12d6361  Trac #18420: fix declaration im polynomial_template.pxi

comment:8 Changed 6 years ago by
Polynomial._mul_trunc_
doctest example should have _mul_trunc_
, not _mul_trunc
comment:9 Changed 6 years ago by
 Commit changed from 12d6361f7dc92fd0054da2b83bab60c59a3ad201 to febe298b5c00d5a5ed521a859e828bbbf2db5421
Branch pushed to git repo; I updated commit sha1. New commits:
febe298  Trac #18420: _mul_trunc > _mul_trunc_ in doctest

comment:10 Changed 6 years ago by
Why did you add the comment in
cpdef ModuleElement _rmul_(self, RingElement c) # ??!?
comment:11 Changed 6 years ago by
 Status changed from needs_review to needs_info
comment:12 Changed 6 years ago by
Because this has nothing to do in sage/rings/polynomial/polynomial_modn_dense_ntl.pxd
. Moreover, it seems to be never used.
It is a good idea to introduce special rule for multiplication by constants. But this should be done globally. Depending on your taste I can either:
 make the comment clearer
 get rid of
_lmul_
and_rmul_
Vincent
comment:13 Changed 6 years ago by
All right, I can just get rid of the declaration in the pxd file and everything is fine. Let me do it.
comment:14 Changed 6 years ago by
 Commit changed from febe298b5c00d5a5ed521a859e828bbbf2db5421 to 063516e84406367fdce86bdc7bf5e19cd9ec14ae
Branch pushed to git repo; I updated commit sha1. New commits:
063516e  Trac #18420: remove redundant declaration of _lmul_, _rmul_

comment:15 Changed 6 years ago by
 Reviewers set to Mario Pernici
 Status changed from needs_info to needs_review
comment:16 Changed 6 years ago by
 Status changed from needs_review to positive_review
comment:17 Changed 6 years ago by
 Branch changed from u/vdelecroix/18420 to 063516e84406367fdce86bdc7bf5e19cd9ec14ae
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Trac 18420: _mul_trunc_ for all polynomials