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

Computing log(0, 2) takes a long time #29164

Closed
cyrilbouvier opened this issue Feb 7, 2020 · 18 comments
Closed

Computing log(0, 2) takes a long time #29164

cyrilbouvier opened this issue Feb 7, 2020 · 18 comments

Comments

@cyrilbouvier
Copy link
Contributor

The following commands run "instantly":

sage: log (0.0)
-infinity
sage: log (0.0, 2)
-infinity
sage: log (0)
-Infinity

But the command

sage: log (0, 2)

takes a very long time (at least a few minutes). Moreover, the python process needs to be killed, using Ctrl+C does not work to stop the computation.

I was able to reproduce the bug with Sage 9.0 (using Python3) and Sage 8.9 (using Python2). But i was not able to reproduce the bug with Sage 7.6.

Component: basic arithmetic

Keywords: log float integer

Author: Ben Livingston

Branch/Commit: e44937e

Reviewer: Frédéric Chapoton

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 14, 2020

comment:1

Batch modifying tickets that will likely not be ready for 9.1, based on a review of the ticket title, branch/review status, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.1, sage-9.2 Apr 14, 2020
@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Aug 29, 2020
@bmlivin
Copy link
Mannequin

bmlivin mannequin commented Sep 5, 2020

comment:4

In sage.rings.integer.Integer.exact_log, this triggers a ValueError:

if mpz_sgn(self.value) <= 0 or mpz_sgn(_m.value) <= 0:
    raise ValueError("both self and m must be positive")

In your example, self.value is 0 and _m.value is 2. It seems to me like it would be better to return -infinity instead of raising this error if self.value is 0, but in a sense that is beside the point. What instead happens is that this ValueError is in a try-except in sage.functions.log.log:

    try:
        return args[0].log(args[1])
    except ValueError as ex:
        if repr(ex)[12:27] == "No discrete log":
            raise
        return logb(args[0], args[1])

Here, args[0] is 0 and args[1] is 2. Calling the function logb results in the same ValueError and the same try-except, and thus an infinite loop.

To avoid this, -infinity needs to be returned at some point. I think that point should be in sage.rings.integer.Integer.exact_log, immediately before raising the ValueError.

@bmlivin bmlivin mannequin self-assigned this Sep 5, 2020
@bmlivin
Copy link
Mannequin

bmlivin mannequin commented Sep 6, 2020

Branch: u/bmlivin/29164

@bmlivin
Copy link
Mannequin

bmlivin mannequin commented Sep 6, 2020

Changed branch from u/bmlivin/29164 to u/gh-bmlivin/29164

@bmlivin
Copy link
Mannequin

bmlivin mannequin commented Sep 6, 2020

Changed branch from u/gh-bmlivin/29164 to none

@bmlivin
Copy link
Mannequin

bmlivin mannequin commented Sep 12, 2020

@bmlivin
Copy link
Mannequin

bmlivin mannequin commented Sep 12, 2020

Commit: fe8c387

@bmlivin
Copy link
Mannequin

bmlivin mannequin commented Sep 12, 2020

New commits:

0f35da0Fixed issue when taking log(0, n) for integer n
fe8c387Added a doctest to sage.functions.log.log to check that issue 29164 is fixed

@bmlivin bmlivin mannequin added the s: needs review label Sep 12, 2020
@fchapoton
Copy link
Contributor

comment:11

in the doctest, please add the missing empty line after ::

+    
+    Check if :track:`29164` is fixed::
+        sage: log(0, 2)
+        -Infinity

@fchapoton
Copy link
Contributor

comment:12

also use :trac: and not :track:

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 13, 2020

Changed commit from fe8c387 to e44937e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 13, 2020

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

e44937eadding empty line and changing track to trac in doctest of sage.functions.log.log

@slel
Copy link
Member

slel commented Sep 14, 2020

comment:14

Full names in "Authors" and "Reviewers" fields please.

@fchapoton
Copy link
Contributor

Reviewer: Frédéric Chapoton

@fchapoton
Copy link
Contributor

comment:16

Ben, please add your full real name in the author field here above.

@bmlivin
Copy link
Mannequin

bmlivin mannequin commented Sep 15, 2020

Author: Ben Livingston

@fchapoton
Copy link
Contributor

comment:18

ok, let it be then. Thanks

@vbraun
Copy link
Member

vbraun commented Sep 23, 2020

Changed branch from u/gh-bmlivin/computing_log_0__2__takes_a_long_time to e44937e

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

5 participants