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.

To avoid rebase I put dependencies on #13213 and #13256.

Attachments (1)

trac_14563-Z_to_quadratic_field.patch (10.7 KB) - added by vdelecroix 6 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 7 years ago by vdelecroix

  • Status changed from new to needs_review

comment:2 Changed 7 years ago by vdelecroix

  • Type changed from PLEASE CHANGE to enhancement

comment:3 Changed 6 years ago by vbraun

There is a block of commented-out code under

# we ignore the code below because of the morphism 

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.

comment:4 Changed 6 years ago by vdelecroix

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 vdelecroix

comment:5 Changed 6 years ago by vbraun

  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

Looks good to me...

comment:6 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:7 Changed 6 years ago by jdemeyer

  • Authors changed from vdelecroix to ​Vincent Delecroix

comment:8 Changed 6 years ago by jdemeyer

  • Merged in set to sage-5.12.beta1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.