Opened 3 years ago

Closed 3 years ago

#19825 closed enhancement (fixed)

Better comparison and conversion code for UCF

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-7.0
Component: number fields Keywords:
Cc: tscrim Merged in:
Authors: Vincent Delecroix Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 9735abb (Commits) Commit: 9735abbddaca5db6ce17ad405587043ac1dfcc08
Dependencies: Stopgaps:

Description (last modified by vdelecroix)

We fasten the comparison code and fix the following for the Universal cyclotomic field: 

sage: CIF(E(3))
Traceback (most recent call last):
...
TypeError: unable to coerce to a ComplexIntervalFieldElement

Before 

sage: %timeit cmp(E(3), E(5))
1000 loops, best of 3: 1.4 ms per loop
sage: %timeit cmp(E(3)+E(3)**2, E(5)+E(5)**2)
1000 loops, best of 3: 975 µs per loop

After 

sage: %timeit cmp(E(3), E(5))
1000 loops, best of 3: 241 µs per loop
sage: %timeit cmp(E(3)+E(3)**2, E(5)+E(5)**2)
1000 loops, best of 3: 181 µs per loop

Change History (5)

comment:1 Changed 3 years ago by vdelecroix

  • Branch set to u/vdelecroix/19825
  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 3 years ago by git

  • Commit set to fc17c996d1c5a9ff1d05fa8929348ea2e8c8fc42

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

​fc17c99Trac 19825: better conversion/comparison in UCF

comment:3 Changed 3 years ago by git

  • Commit changed from fc17c996d1c5a9ff1d05fa8929348ea2e8c8fc42 to 9735abbddaca5db6ce17ad405587043ac1dfcc08

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

​9735abbTrac 19825: a doctest fix + two doctests

comment:4 Changed 3 years ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Even though it looks like the float lost precision, it is in fact closer to the actual value: 

sage: float(1.002000)  # Trailing 0's are removed
1.002
sage: x = E(7) + E(7,6)
sage: x.n(1000)
1.2469796037174670610500097680084...

So LGTM.

comment:5 Changed 3 years ago by vbraun

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