Opened 7 years ago
Closed 6 years ago
#14563 closed enhancement (fixed)
faster coercion from Integer to Quadratic Number fields
Reported by: | vdelecroix | Owned by: | vdelecroix |
---|---|---|---|
Priority: | major | Milestone: | sage-5.12 |
Component: | coercion | Keywords: | quadratic number field, coercion |
Cc: | Merged in: | sage-5.12.beta1 | |
Authors: | Vincent Delecroix | Reviewers: | Volker Braun |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #13213, #13256 | Stopgaps: |
Description
As suggested in this thread on sage-devel, the patch implements a faster coercion from the integers to quadratic number fields.
Attachments (1)
Change History (9)
comment:1 Changed 7 years ago by
- Status changed from new to needs_review
comment:2 Changed 7 years ago by
- Type changed from PLEASE CHANGE to enhancement
comment:3 Changed 6 years ago by
comment:4 Changed 6 years ago by
Thanks for having a look.
I commented the code because it was never used because of the coercion system (see the __call__
of Parent). It was the case even before my patch. I think the gain is minor but the fact is that it was unused. I will remove it.
Here are some timings, the improvement is somwhere around x10.
without
sage: K=QuadraticField(2) sage: q = 12 sage: %timeit K(q) 100000 loops, best of 3: 10.3 us per loop sage: g = K.gen() sage: %timeit q*g 1000000 loops, best of 3: 1.66 us per loop sage: %timeit q+g 100000 loops, best of 3: 13.2 us per loop
with
sage: K=QuadraticField(2) sage: q = 12 sage: %timeit K(q) 1000000 loops, best of 3: 660 ns per loop sage: g = K.gen() sage: %timeit q*g 1000000 loops, best of 3: 1.74 us per loop sage: %timeit q+g 100000 loops, best of 3: 2.76 us per loop
There is no difference for the multiplication because the methods _lmul_
or _rmul_
are called directly and there is not a call to the coercion morphism.
Changed 6 years ago by
comment:5 Changed 6 years ago by
- Reviewers set to Volker Braun
- Status changed from needs_review to positive_review
Looks good to me...
comment:6 Changed 6 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:7 Changed 6 years ago by
comment:8 Changed 6 years ago by
- Merged in set to sage-5.12.beta1
- Resolution set to fixed
- Status changed from positive_review to closed
There is a block of commented-out code under
If its not used then delete it, don't leave commented-out chunks of code lying around. Is there a performance difference with this branch removed? It looks like an attempt to deliberately circumvent the morphism to me.