Opened 11 years ago
Closed 11 years ago
#7562 closed defect (fixed)
Another (new?) binomial bug
Reported by: | kcrisman | Owned by: | AlexGhitza |
---|---|---|---|
Priority: | major | Milestone: | sage-4.3 |
Component: | basic arithmetic | Keywords: | |
Cc: | hgranath | Merged in: | sage-4.3.rc0 |
Authors: | Håkan Granath | Reviewers: | Karl-Dieter Crisman |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
From sage-devel:
In [143]: [binomial(1,1),binomial(1,2),binomial(1,3),binomial(1,4)] Out[143]: [1, 0, 0, 0] In [144]: [binomial(1.0,1),binomial(1.0,2),binomial(1.0,3),binomial (1.0,4)] Out[144]: [1.00000000000000, 0.000000000000000, NaN, NaN]
The problem is this:
sage: x = RealNumber('1.0') sage: P = x.parent() sage: P Real Field with 53 bits of precision sage: gamma(x+1)/gamma(P(Integer(4)+1))/gamma(x-Integer(4)+1) NaN sage: gamma(x-Integer(4)+1) NaN
So we'll have to put in yet another check...
Attachments (1)
Change History (10)
comment:1 follow-up: ↓ 3 Changed 11 years ago by
- Status changed from new to needs_review
comment:2 Changed 11 years ago by
- Status changed from needs_review to needs_work
Sorry, my patch is off for negative x. I will send an updated patch later.
comment:3 in reply to: ↑ 1 ; follow-up: ↓ 4 Changed 11 years ago by
By the way, is the cc field above supposed to notify me by mail? I did not get any.
Yes, and it does usually work, but perhaps you don't have a correct email associated to your account. I don't know how to fix this, you may want to ask on sage-devel.
comment:4 in reply to: ↑ 3 Changed 11 years ago by
Replying to kcrisman:
Yes, and it does usually work, but perhaps you don't have a correct email associated to your account. I don't know how to fix this, you may want to ask on sage-devel.
Thanks for the info, I found some place to enter my email so probably it will work now.
comment:5 Changed 11 years ago by
- Status changed from needs_work to needs_review
comment:6 Changed 11 years ago by
- Reviewers set to Karl-Dieter Crisman
- Status changed from needs_review to needs_work
This seems to work fine, and agrees with integer calculations. Obviously passes tests.
But:
sage: binomial(0,0) 1 sage: binomial(0.,0) NaN
I don't know which is the usual convention, but they should probably agree.
comment:7 Changed 11 years ago by
- Status changed from needs_work to needs_review
Thanks for spotting this, new version is up.
comment:8 Changed 11 years ago by
- Status changed from needs_review to positive_review
Great, positive review!
comment:9 Changed 11 years ago by
- Merged in set to sage-4.3.rc0
- Resolution set to fixed
- Status changed from positive_review to closed
Here is a patch that should fix this issue. Note that for x a negative floating point integer we hit gamma function poles both in the numerator and denominator. An alternative formula is used in the patch to avoid this.
By the way, is the cc field above supposed to notify me by mail? I did not get any.