Opened 7 years ago
Closed 5 years ago
#19444 closed defect (fixed)
sage.misc.functional.log(float(3)) raises an AttributeError
Reported by: | slabbe | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.1 |
Component: | basic arithmetic | Keywords: | |
Cc: | tscrim, jdemeyer, jhpalmieri | Merged in: | |
Authors: | Frédéric Chapoton | Reviewers: | Sébastien Labbé |
Report Upstream: | N/A | Work issues: | |
Branch: | d2f7962 (Commits, GitHub, GitLab) | Commit: | d2f7962a6ee8a2e6e0b174083a44b4ed067b5785 |
Dependencies: | Stopgaps: |
Description
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
?
Change History (26)
comment:1 Changed 5 years ago by
- Branch set to u/chapoton/19444
- Cc tscrim jdemeyer jhpalmieri added
- Commit set to f4c012de47d26ab2fabb24266805ac6dccd5a8cb
- Milestone changed from sage-6.10 to sage-8.1
- Status changed from new to needs_review
comment:2 Changed 5 years ago by
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
comment:3 Changed 5 years ago by
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.
comment:4 Changed 5 years ago by
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.
comment:5 Changed 5 years ago by
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...
comment:6 Changed 5 years ago by
And indeed, globals()
is restricted to the module where the function is defined... so that its usage is wrong.
comment:7 Changed 5 years ago by
See #23779 for the fix to import_statements
comment:8 Changed 5 years ago by
- Commit changed from f4c012de47d26ab2fabb24266805ac6dccd5a8cb to 02e1fc084228ff509b6927ce82190acb55426caf
comment:9 Changed 5 years ago by
ok, done
comment:10 Changed 5 years ago by
- Commit changed from 02e1fc084228ff509b6927ce82190acb55426caf to b1a85c98b3d957bc23f9002de7db6cc54e52dce0
Branch pushed to git repo; I updated commit sha1. New commits:
b1a85c9 | trac 19444 fixing another log import
|
comment:11 Changed 5 years ago by
- Reviewers set to Sébastien Labbé
- Status changed from needs_review to needs_work
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
comment:12 Changed 5 years ago by
- Commit changed from b1a85c98b3d957bc23f9002de7db6cc54e52dce0 to 465125e6133b5f33493d1f78b98237228bb7dae4
Branch pushed to git repo; I updated commit sha1. New commits:
465125e | trac 19444 better deprecation message
|
comment:14 Changed 5 years ago by
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.
comment:15 Changed 5 years ago by
- Status changed from needs_review to needs_work
comment:16 Changed 5 years ago by
I would replace ceil(log(n,2))
by (ZZ(n) - 1).nbits()
which is guaranteed to be correct and reasonably fast.
comment:17 Changed 5 years ago by
- Commit changed from 465125e6133b5f33493d1f78b98237228bb7dae4 to e09327389baa0b96060ac5ea36cf77b69ee6dab6
comment:18 Changed 5 years ago by
ok ; but now there remains an RDF in "src/sage/rings/finite_rings/element_ntl_gf2e.pyx"
comment:19 Changed 5 years ago by
- Commit changed from e09327389baa0b96060ac5ea36cf77b69ee6dab6 to 5489453100b096923d1376b82139c4d3952edd83
comment:21 Changed 5 years ago by
In the light of __future__ division
, it is better to use // 8
instead of / 8
.
comment:22 Changed 5 years ago by
- Commit changed from 5489453100b096923d1376b82139c4d3952edd83 to d2f7962a6ee8a2e6e0b174083a44b4ed067b5785
Branch pushed to git repo; I updated commit sha1. New commits:
d2f7962 | trac 19444 double //
|
comment:23 Changed 5 years ago by
done
comment:24 Changed 5 years ago by
bot is morally green
comment:25 Changed 5 years ago by
- Status changed from needs_review to positive_review
comment:26 Changed 5 years ago by
- Branch changed from u/chapoton/19444 to d2f7962a6ee8a2e6e0b174083a44b4ed067b5785
- Resolution set to fixed
- Status changed from positive_review to closed
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:
fixing the behaviour of misc.functional.log