Opened 4 years ago

Closed 4 years ago

#18420 closed enhancement (fixed)

Uniformize truncated multiplication for polynomials

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-6.7
Component: algebra Keywords:
Cc: Merged in:
Authors: Vincent Delecroix Reviewers: Mario Pernici
Report Upstream: N/A Work issues:
Branch: 063516e (Commits) Commit: 063516e84406367fdce86bdc7bf5e19cd9ec14ae
Dependencies: Stopgaps:

Description (last modified by vdelecroix)

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 4 years ago by vdelecroix

  • Description modified (diff)

comment:2 Changed 4 years ago by vdelecroix

  • Branch set to u/vdelecroix/18420
  • Commit set to 446b952f32665697ee168f7fc61649f540a82e73
  • Status changed from new to needs_review

New commits:

446b952Trac 18420: _mul_trunc_ for all polynomials

comment:3 Changed 4 years ago by git

  • Commit changed from 446b952f32665697ee168f7fc61649f540a82e73 to 4e96421ff4be51ed7ff188281a0764f7c1a6ba7c

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

4e96421Trac 18420: _mul_trunc_ for all polynomials

comment:4 Changed 4 years ago by git

  • Commit changed from 4e96421ff4be51ed7ff188281a0764f7c1a6ba7c to b73a8404319fc0f7542989c340376c43610bc1e0

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

b73a840Trac 18420: _mul_trunc_ for all polynomials

comment:5 Changed 4 years ago by vdelecroix

rebased on 6-7.rc0

comment:6 Changed 4 years ago by vdelecroix

  • Description modified (diff)

comment:7 Changed 4 years ago by git

  • Commit changed from b73a8404319fc0f7542989c340376c43610bc1e0 to 12d6361f7dc92fd0054da2b83bab60c59a3ad201

Branch pushed to git repo; I updated commit sha1. New commits:

12d6361Trac #18420: fix declaration im polynomial_template.pxi

comment:8 Changed 4 years ago by pernici

Polynomial._mul_trunc_ doctest example should have _mul_trunc_, not _mul_trunc

comment:9 Changed 4 years ago by git

  • Commit changed from 12d6361f7dc92fd0054da2b83bab60c59a3ad201 to febe298b5c00d5a5ed521a859e828bbbf2db5421

Branch pushed to git repo; I updated commit sha1. New commits:

febe298Trac #18420: _mul_trunc -> _mul_trunc_ in doctest

comment:10 Changed 4 years ago by pernici

Why did you add the comment in cpdef ModuleElement _rmul_(self, RingElement c) # ??!?

comment:11 Changed 4 years ago by pernici

  • Status changed from needs_review to needs_info

comment:12 Changed 4 years ago by vdelecroix

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. I will make the comment clearer.

Vincent

Last edited 4 years ago by vdelecroix (previous) (diff)

comment:13 Changed 4 years ago by vdelecroix

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 4 years ago by git

  • Commit changed from febe298b5c00d5a5ed521a859e828bbbf2db5421 to 063516e84406367fdce86bdc7bf5e19cd9ec14ae

Branch pushed to git repo; I updated commit sha1. New commits:

063516eTrac #18420: remove redundant declaration of _lmul_, _rmul_

comment:15 Changed 4 years ago by vdelecroix

  • Reviewers set to Mario Pernici
  • Status changed from needs_info to needs_review

comment:16 Changed 4 years ago by pernici

  • Status changed from needs_review to positive_review

comment:17 Changed 4 years ago by vbraun

  • Branch changed from u/vdelecroix/18420 to 063516e84406367fdce86bdc7bf5e19cd9ec14ae
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.