Opened 10 years ago
Closed 8 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 10 years ago by
Type:  PLEASE CHANGE → defect 

comment:2 Changed 9 years ago by
Cc:  benjaminfjones,burcin, → benjaminfjones, burcin 

Milestone:  sage5.11 → sage5.12 
comment:3 Changed 9 years ago by
Cc:  eviatarbach added 

comment:4 Changed 9 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 9 years ago by
Milestone:  sage6.1 → sage6.2 

comment:6 Changed 9 years ago by
Milestone:  sage6.2 → sage6.3 

comment:7 Changed 8 years ago by
Milestone:  sage6.3 → sage6.4 

comment:8 Changed 8 years ago by
Branch:  → u/jdemeyer/ticket/14766 

Created:  Jun 18, 2013, 5:37:43 PM → Jun 18, 2013, 5:37:43 PM 
Modified:  Aug 10, 2014, 4:51:03 PM → Aug 10, 2014, 4:51:03 PM 
comment:9 Changed 8 years ago by
Authors:  → Jeroen Demeyer 

Commit:  → 68c545a5edb07ea87adea5850a143195723dfced 
Dependencies:  → #17130 
Status:  new → 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 8 years ago by
Reviewers:  → Ralf Stephan 

Status:  needs_review → positive_review 
Looks fine and passes tests in functions/
.
comment:11 Changed 8 years ago by
Branch:  u/jdemeyer/ticket/14766 → 68c545a5edb07ea87adea5850a143195723dfced 

Resolution:  → fixed 
Status:  positive_review → 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.