sage.misc.functional.log(float(3)) raises an AttributeError

[seblabbe]

import_statements suggest to import log this way:

sage: import_statements('log')
# **Warning**: distinct objects with name 'log' in:
#   - sage.functions.log
#   - math
#   - sage.functions
#   - sage.misc.functional
from sage.misc.functional import log

While those works:

sage: math.log(float(3))
1.0986122886681098
sage: sage.functions.log.log(float(3))
1.0986122886681098

This one raises an AttributeError:

sage: sage.misc.functional.log(float(3))
Traceback (most recent call last):
...
AttributeError: 'sage.rings.real_double.RealDoubleElement' object has no attribute '_log_base'

A subquestion is why do we have two implementations of log?

CC: @tscrim @jdemeyer @jhpalmieri

Component: basic arithmetic

Author: Frédéric Chapoton

Branch/Commit: d2f7962

Reviewer: Sébastien Labbé

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

[fchapoton]

Author: Frédéric Chapoton

[fchapoton]

comment:1

Done. This should be an easy review.

NOTA BENE: Getting rid of this log function (in favor of the one in functions.log) should be done in another ticket. I tried to do that, but sutmbled on the infamous import hell (import cycles everywhere).

---

New commits:

f4c012d

fixing the behaviour of misc.functional.log

[fchapoton]

Branch: u/chapoton/19444

[fchapoton]

Commit: f4c012d

[videlec]

comment:2

This implementation is very weak. How can you assume that this is supposed to be the real logarithm?

sage: sage.misc.functional.log(complex(3j))
Traceback (most recent call last):
...
TypeError: can't convert complex to float

versus

sage: sage.misc.functional.log(CDF(0,3))
1.0986122886681098 + 1.5707963267948966*I

[fchapoton]

comment:3

There is no need to make another perfect log, there is one already in functions.log, which is the one we provide as "log" in the global namespace. We should fix the issue raised here. And later get rid of this function, which is only used 10 times or so.

[videlec]

comment:4

What is the point of fixing an issue on a useless function? If I open a ticket for the complex case will you fix it ;-? If useless, the log from functional would better be deprecated and this ticket closed as invalid.

Anyway, the complaint is about import_statements that is not fixed by your branch.

[videlec]

comment:5

And indeed, there is something wrong with find_objects_from_name

sage: 
sage: from sage.misc.dev_tools import find_objects_from_name
sage: find_objects_from_name('log')
[<function log at ...>,
 <function log at ...>,
 log,
 <built-in function log>,
 <module 'sage.functions.log' from '/opt/sage/local/lib/python2.7/site-packages/sage/functions/log.pyc'>]

The above is wrong since log is in the global namespace and the answer should have been the log from the global namespace...

[videlec]

comment:6

And indeed, globals() is restricted to the module where the function is defined... so that its usage is wrong.

[videlec]

comment:7

See #23779 for the fix to import_statements

[sagetrac-git]

Changed commit from f4c012d to 02e1fc0

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

b0aa14f	test
02e1fc0	trac 19444 avoid circular imports

[fchapoton]

comment:9

ok, done

[sagetrac-git]

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

b1a85c9

trac 19444 fixing another log import

[sagetrac-git]

Changed commit from 02e1fc0 to b1a85c9

[seblabbe]

comment:11

If it turns out that for some reason I have used the bad log function in my code and I now get the following error message:

DeprecationWarning: this version of log is no longer used

I will not understand and I will lose time to understand what is happening.

I much prefer

DeprecationWarning: function sage.misc.functional.log is deprecated, use sage.functions.log or log from the global sage namespace instead

[seblabbe]

Reviewer: Sébastien Labbé

[sagetrac-git]

Changed commit from b1a85c9 to 465125e

[sagetrac-git]

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

465125e

trac 19444 better deprecation message

[fchapoton]

comment:13

Done

[jdemeyer]

comment:14

This is wrong:

-        k = int(ceil(log(n,2)))
+        k = int(ceil(RDF(n).log(2)))

Do not use floating-point computations when you want an exact answer.

An exception to this would be MPFR (used in Sage by RR) because that does give a guarantee of exactness if possible.

So, you should either use ZZ or RR to do this computation.

[jdemeyer]

comment:16

I would replace ceil(log(n,2)) by (ZZ(n) - 1).nbits() which is guaranteed to be correct and reasonably fast.

[sagetrac-git]

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

59f1ee4	trac 19444 using nbits
e093273	Merge branch 'u/chapoton/19444'

[sagetrac-git]

Changed commit from 465125e to e093273

[fchapoton]

comment:18

ok ; but now there remains an RDF in "src/sage/rings/finite_rings/element_ntl_gf2e.pyx"

[sagetrac-git]

Changed commit from e093273 to 5489453

[sagetrac-git]

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

ed4e22a	Merge branch 'u/chapoton/19444' in 8.1.b4
5489453	trac 19444 do not use RDF().log()

[fchapoton]

comment:20

should be good now

[jdemeyer]

comment:21

In the light of __future__ division, it is better to use // 8 instead of / 8.

[sagetrac-git]

Changed commit from 5489453 to d2f7962

[sagetrac-git]

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

d2f7962

trac 19444 double //

[fchapoton]

comment:23

done

[fchapoton]

comment:24

bot is morally green

[vbraun]

Changed branch from u/chapoton/19444 to d2f7962