Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance of casting polynomials to polynomials over finite fields #15427

Open
sagetrac-afiori mannequin opened this issue Nov 16, 2013 · 2 comments
Open

Performance of casting polynomials to polynomials over finite fields #15427

sagetrac-afiori mannequin opened this issue Nov 16, 2013 · 2 comments

Comments

@sagetrac-afiori
Copy link
Mannequin

sagetrac-afiori mannequin commented Nov 16, 2013

One would expect the performance of casting the usual way (as in test 1 below) to be at least not much worse than test 2:

sage: QQX=QQ['x']
sage: QP=Qp(3,30);
sage: R=QP.residue_field();
sage: RX=R['x'];
sage: prim=primes_first_n(1000)
sage: polysQQ = [ QQX(prim[i:i+30]) for i in range(1,900)]
sage: def test1(PR,l):
        return [PR(P) for P in l];
....:
sage: def test2(PR,l):
        return [PR([PR.base_ring()(coef) for coef in P.list()]) for P in l];
....:
sage: %timeit test1(RX,polysQQ)
1 loops, best of 3: 478 ms per loop
sage: %timeit test2(RX,polysQQ)
1 loops, best of 3: 230 ms per loop

Especially since the actual implementation of casting that is performed essentially reduces to converting to a list and casting as we do in test 2.

The problem is that the implementation of list->polynomial casting provided by Polynomial_template is very much not optimal, sufficiently so that two of the three implementing classes handle lists in their own __init__ rather than passing through to polynomial template.

The polynomial->polynomial casting is then also inefficient as it converts the poly to a list and recalls init. In the original code this bypassed the optimized list implementations in the implementing class, this is what the current patch changes. The effect on performance is as follows:

sage: %timeit test1(RX,polysQQ)
1 loops, best of 3: 219 ms per loop

There is still room for improvement, the current (and with patch) implementation will in most cases end up calling Polynomial_template.init twice, and likely also Polynomial.init twice, this is wasteful (even though the calls are largely no-ops). It would probably be better to, in addition to the currently proposed patch, special case casts from polynomials in the implementing classes the same way lists are done to avoid this.

CC: @roed314 @jpflori

Component: performance

Keywords: FiniteField Polynomial Casting

Author: Andrew Fiori

Issue created by migration from https://trac.sagemath.org/ticket/15427

@sagetrac-afiori sagetrac-afiori mannequin added this to the sage-6.1 milestone Nov 16, 2013
@sagetrac-afiori
Copy link
Mannequin Author

sagetrac-afiori mannequin commented Nov 16, 2013

Attachment: FiniteFieldPolyCast.patch.gz

@sagetrac-afiori
Copy link
Mannequin Author

sagetrac-afiori mannequin commented Nov 17, 2013

Author: Andrew Fiori

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@mkoeppe mkoeppe removed this from the sage-6.4 milestone Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant