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

Rational log not working #21429

Closed
rwst opened this issue Sep 6, 2016 · 19 comments
Closed

Rational log not working #21429

rwst opened this issue Sep 6, 2016 · 19 comments

Comments

@rwst
Copy link

rwst commented Sep 6, 2016

sage: ZZ(8).log(2)
3

sage: QQ(8/27).log(2/3)
...
AttributeError: 'sage.rings.rational.Rational' object has no attribute 'log'

This can be done fast using mpz_remove (or ZZ.log) on numerator and denominator.

Depends on #21517

Component: numerical

Author: Ralf Stephan

Branch/Commit: 8c2ad23

Reviewer: Travis Scrimshaw

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

@rwst rwst added this to the sage-7.4 milestone Sep 6, 2016
@rwst
Copy link
Author

rwst commented Sep 28, 2016

Dependencies: #21518

@rwst
Copy link
Author

rwst commented Sep 29, 2016

Changed dependencies from #21518 to #21517

@rwst
Copy link
Author

rwst commented Sep 29, 2016

Branch: u/rws/21429-1

@rwst
Copy link
Author

rwst commented Sep 29, 2016

Commit: a16e1c4

@rwst
Copy link
Author

rwst commented Sep 29, 2016

Author: Ralf Stephan

@rwst
Copy link
Author

rwst commented Sep 29, 2016

New commits:

de1acfa21517: handle ZZ.log(1/n)
4aba8a921517: avoid duplicate computations
a16e1c421429: implement rational log

@tscrim
Copy link
Collaborator

tscrim commented Sep 29, 2016

comment:6

Some micro-optimizations (I slightly care because this could be used in tight loops):

  • You call Rational(m) twice.

  • You call the perfect_power of anum and the like twice.

  • an (and other like variables) are integers, correct? IIRC, a faster way to construct rational numbers is Rational(an, bn).

  • It is likely faster to extract the numerator and denominator than to use a try/except block:

    sage: def foo(q):
    ....:     try:
    ....:         return ZZ(q)
    ....:     except (ValueError, TypeError):
    ....:         return None
    sage: def bar(q):
    ....:     if q.denom() == ZZ.one():
    ....:         return q.numer()
    ....:     else:
    ....:         return None
    sage: q = 4 / 3
    sage: %timeit foo(q)
    The slowest run took 7.89 times longer than the fastest. This could mean that an intermediate result is being cached.
    1000000 loops, best of 3: 1.24 µs per loop
    sage: %timeit bar(q)
    The slowest run took 42.67 times longer than the fastest. This could mean that an intermediate result is being cached.
    1000000 loops, best of 3: 212 ns per loop
    sage: q = 4 / 1
    sage: %timeit foo(q)
    The slowest run took 35.47 times longer than the fastest. This could mean that an intermediate result is being cached.
    1000000 loops, best of 3: 202 ns per loop
    sage: %timeit bar(q)
    The slowest run took 37.56 times longer than the fastest. This could mean that an intermediate result is being cached.
    1000000 loops, best of 3: 292 ns per loop

    You might be able to get even a bit more out of this by not going through numer and denom, but using the actual mpir data.

A cosmetic change:

-        -  ``m`` - default: natural log base e
+        -  ``m`` -- (default: natural log base `e`) the base
 
-        -  ``prec`` - integer (default: None): if None, return
+        -  ``prec`` -- integer (default: ``None``); if ``None``, return
            symbolic, else to given bits of precision as in RealField

@rwst
Copy link
Author

rwst commented Sep 30, 2016

Changed branch from u/rws/21429-1 to u/rws/21429-2

@rwst
Copy link
Author

rwst commented Sep 30, 2016

comment:8

This version also looks much better.


New commits:

997afa821429: rational log

@rwst
Copy link
Author

rwst commented Sep 30, 2016

Changed commit from a16e1c4 to 997afa8

@cheuberg
Copy link
Contributor

comment:9

failing doctests (see patchbot).

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 15, 2017

Changed commit from 997afa8 to 2ded570

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 15, 2017

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

95e8048Merge branch 'develop' into t/21429/21429-2
2ded57021429: fix doctest fails

@rwst rwst modified the milestones: sage-7.4, sage-7.6 Jan 15, 2017
@tscrim
Copy link
Collaborator

tscrim commented Jan 15, 2017

Changed branch from u/rws/21429-2 to u/tscrim/rational_log_fix-21429

@tscrim
Copy link
Collaborator

tscrim commented Jan 15, 2017

comment:12

Sorry for having this fall off my radar. I made some fixes to the doc and some other small cosmetic changes. If you agree, then you can set a positive review.


New commits:

8c2ad23Docstring and cosmetic changes.

@tscrim
Copy link
Collaborator

tscrim commented Jan 15, 2017

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jan 15, 2017

Changed commit from 2ded570 to 8c2ad23

@rwst
Copy link
Author

rwst commented Jan 16, 2017

comment:13

Thanks!

@vbraun
Copy link
Member

vbraun commented Jan 21, 2017

Changed branch from u/tscrim/rational_log_fix-21429 to 8c2ad23

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

4 participants