Opened 9 years ago

Closed 9 years ago

#12239 closed enhancement (fixed)

Better conversion to/from ECL bignums

Reported by: nbruin Owned by: was
Priority: minor Milestone: sage-5.0
Component: interfaces Keywords: ecl, maxima
Cc: Merged in: sage-5.0.beta1
Authors: Nils Bruin Reviewers: Burcin Erocal
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by burcin)

Previously, python <-> ECL conversion of bigints went via (decimal!) string representation. This ticket fixes this. Timings before:

sage: from sage.libs.ecl import *
sage: i = 10^(10^5)
sage: o = EclObject(i)
sage: timeit('EclObject(i)')
5 loops, best of 3: 331 ms per loop
sage: timeit('o.python()')
25 loops, best of 3: 18.6 ms per loop

after:

sage: timeit('EclObject(i)')
625 loops, best of 3: 44.2 µs per loop
sage: timeit('o.python()')
625 loops, best of 3: 4.49 µs per loop

These conversions will benefit symbolic routines that call maxima via the binary interface (integrate, limit, sum) but obviously not the ones that use all-string conversion. Most symbolic work does not involve large integers anyway.

NOTE: Current implementation copies the bitstring twice for python -> ECL. In principle this could be avoided but needs reaching into ECL's internals. The "ecl_big_register" routines are exported, but the other routines are "static". This is because there are some subtleties with how ECL avoids GMPs memory reallocation to step in. Therefore, the present solution is probably the safest. Should this ever become a bottleneck, we can reconsider.

Apply attachment:trac_12239-ecl_bignum.patch

Attachments (2)

trac_12239.patch (5.0 KB) - added by nbruin 9 years ago.
trac_12239-ecl_bignum.patch (4.5 KB) - added by burcin 9 years ago.

Download all attachments as: .zip

Change History (6)

Changed 9 years ago by nbruin

comment:1 Changed 9 years ago by nbruin

  • Status changed from new to needs_review

Changed 9 years ago by burcin

comment:2 Changed 9 years ago by burcin

  • Authors set to Nils Bruin
  • Description modified (diff)
  • Keywords ecl maxima added
  • Reviewers set to Burcin Erocal
  • Status changed from needs_review to positive_review

Patch looks good. Applies cleanly to 4.8.alpha4. Tests pass on symbolics related files (sage/{symbolics,functions,calculus} etc.). Positive review.

I attached a new patch which also removes the comments that refer to the previous string conversion code and say that this can be done more efficiently.

Apply attachment:trac_12239-ecl_bignum.patch only.

comment:3 Changed 9 years ago by jdemeyer

  • Milestone changed from sage-4.8 to sage-5.0

comment:4 Changed 9 years ago by jdemeyer

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