Opened 8 years ago
Closed 6 years ago
#14766 closed defect (fixed)
Fix Python int problem with exp_integral
Reported by:  kcrisman  Owned by:  burcin 

Priority:  major  Milestone:  sage6.4 
Component:  symbolics  Keywords:  
Cc:  benjaminfjones, burcin, eviatarbach  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Ralf Stephan 
Report Upstream:  N/A  Work issues:  
Branch:  68c545a (Commits, GitHub, GitLab)  Commit:  68c545a5edb07ea87adea5850a143195723dfced 
Dependencies:  #17130  Stopgaps: 
Description
In #11143 we weren't careful about Python ints. There are lots of examples of this, so one will want to go through the whole file.
sage: exp_integral_e(int(3),0) 0 sage: exp_integral_e(3,0) 1/2
Change History (11)
comment:1 Changed 8 years ago by
 Type changed from PLEASE CHANGE to defect
comment:2 Changed 8 years ago by
 Cc changed from benjaminfjones,burcin, to benjaminfjones, burcin
 Milestone changed from sage5.11 to sage5.12
comment:3 Changed 8 years ago by
 Cc eviatarbach added
comment:4 Changed 8 years ago by
I think my preference would be either raising a TypeError
(and clearly documenting what the allowed types are) or doing Python 3 style division. I don't like the idea of adding an implicit coercion int
> Integer
.
If we do type checking in BuiltinFunction.__call__
, we should make sure that there isn't a significant performance penalty.
comment:5 Changed 7 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:6 Changed 7 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:7 Changed 7 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:8 Changed 6 years ago by
 Branch set to u/jdemeyer/ticket/14766
 Created changed from 06/18/13 17:37:43 to 06/18/13 17:37:43
 Modified changed from 08/10/14 16:51:03 to 08/10/14 16:51:03
comment:9 Changed 6 years ago by
 Commit set to 68c545a5edb07ea87adea5850a143195723dfced
 Dependencies set to #17130
 Status changed from new to needs_review
New commits:
6d10729  Simplify numerical evaluation of BuiltinFunctions

b6e1ed4  Merge remotetracking branches 'origin/u/jdemeyer/ticket/17131' and 'origin/u/jdemeyer/ticket/17133' into ticket/17130

382f97a  Merge branch 'u/jdemeyer/ticket/17130' of trac.sagemath.org:sage into 6.5beta1

7265989  17130: reviewer's patch: fix typo

c47dbd4  Fix Trac #17328 again in a better way

a486db2  Call the factorial() method of an Integer

68c545a  Fix bugs due to Python 2 int division

comment:10 Changed 6 years ago by
 Reviewers set to Ralf Stephan
 Status changed from needs_review to positive_review
Looks fine and passes tests in functions/
.
comment:11 Changed 6 years ago by
 Branch changed from u/jdemeyer/ticket/14766 to 68c545a5edb07ea87adea5850a143195723dfced
 Resolution set to fixed
 Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
I'm guessing this occurs in other files too.
I think it's unreasonable to expect everyone adding symbolic functions to worry about this, and certainly users to be aware of it; maybe we could make
BuiltinFunction.__call__
automatically convertint
toInteger
? Or have it raise an error if anint
is in the parameters? Allowingint
into_eval_
causes other problems too, such as getting expressions withint
pyobjects in them, making arithmetic slower, etc.At the very least, we could temporarily add
from __future__ import division
in all files that implement symbolic functions; then exact answers wouldn't be returned in all cases where they should, but at least mathematically wrong answers wouldn't occur.