Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add kernel implementation of Binomial() #1921

Merged
merged 1 commit into from
Nov 21, 2017

Conversation

fingolfin
Copy link
Member

This example illustrates the speedup:

gap> n:=2^14;; a:=BINOMIAL_INT(2*n,n);; time; b:=Binomial_GAP(2*n,n);; time; a=b;
0
51
true
gap> n:=2^16;; a:=BINOMIAL_INT(2*n,n);; time; b:=Binomial_GAP(2*n,n);; time; a=b;
2
719
true
gap> n:=2^18;; a:=BINOMIAL_INT(2*n,n);; time; b:=Binomial_GAP(2*n,n);; time; a=b;
8
11470
true
gap> n:=2^19;; a:=BINOMIAL_INT(2*n,n);; time; b:=Binomial_GAP(2*n,n);; time; a=b;
14
51386
gap> n:=2^20;; a:=BINOMIAL_INT(2*n,n);; time; b:=Binomial_GAP(2*n,n);; time; a=b;
33
190444

Note that on 32bit systems, some previously "supported" combinations (n,k) are no longer supported. But those values are effectively out of reach for the old code, too: if I extrapolate the results above in various ways, I get an estimate of about 30 days to compute the smallest binomial coefficient (for n=2^28) not supported. Granted, this is on my laptop, and your super computer might get to it quicker. But at that point you might also just use the 64 bit version of GAP, which gets the result on my laptop in less than 30 seconds:

gap> n:=2^28;; a:=BINOMIAL_INT(2*n,n);; time;
26180
gap> MemoryUsage(a);
67108888
gap> StringOfMemoryAmount(last);
"64.0MB"

@fingolfin fingolfin added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: kernel labels Nov 17, 2017
Copy link
Member

@markuspf markuspf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why you keep the old GAP code around (commented out)?

@codecov
Copy link

codecov bot commented Nov 17, 2017

Codecov Report

Merging #1921 into master will increase coverage by 0.05%.
The diff coverage is 87.09%.

@@            Coverage Diff             @@
##           master    #1921      +/-   ##
==========================================
+ Coverage   63.71%   63.77%   +0.05%     
==========================================
  Files         947      947              
  Lines      284484   284539      +55     
  Branches    12744    12757      +13     
==========================================
+ Hits       181264   181457     +193     
+ Misses     100436   100284     -152     
- Partials     2784     2798      +14
Impacted Files Coverage Δ
lib/combinat.gi 66.53% <0%> (-1.17%) ⬇️
src/integer.c 87.78% <88.52%> (+0.85%) ⬆️
src/hpc/thread.c 45.41% <0%> (-0.6%) ⬇️
src/stats.c 78.88% <0%> (-0.41%) ⬇️
hpcgap/lib/hpc/stdtasks.g 38.61% <0%> (-0.26%) ⬇️
src/funcs.c 77.41% <0%> (-0.15%) ⬇️
src/hpc/traverse.c 78.29% <0%> (-0.09%) ⬇️
src/gap.c 58.67% <0%> (-0.08%) ⬇️
src/hpc/threadapi.c 34.64% <0%> (+0.09%) ⬆️
... and 5 more

This uses the GMP functions mpz_bin_ui and mpz_bin_uiui. The following timings
for certain central binomial coefficients illustrates the speedup:

gap> n:=2^14;; a:=BINOMIAL_INT(2*n,n);; time; b:=Binomial_GAP(2*n,n);; time;
0
51

gap> n:=2^16;; a:=BINOMIAL_INT(2*n,n);; time; b:=Binomial_GAP(2*n,n);; time;
2
719

gap> n:=2^18;; a:=BINOMIAL_INT(2*n,n);; time; b:=Binomial_GAP(2*n,n);; time;
8
11470

gap> n:=2^19;; a:=BINOMIAL_INT(2*n,n);; time; b:=Binomial_GAP(2*n,n);; time;
14
51386

gap> n:=2^20;; a:=BINOMIAL_INT(2*n,n);; time; b:=Binomial_GAP(2*n,n);; time;
33
190444

The main reason the C version is so much faster is that it can accumulate the
result in-place, whereas GAP creates tons and tons of temporary integer
objects which then need to be garbage collected. Of course some other tricks
employed by GMP help, too.
@fingolfin
Copy link
Member Author

Not really, I just forgot to remove it. Now fixed. Also tweaked the commit message a bit.

@ChrisJefferson ChrisJefferson merged commit 7a1953c into gap-system:master Nov 21, 2017
@fingolfin fingolfin deleted the mh/binomial branch November 21, 2017 15:08
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.9.0 milestone Jan 22, 2018
@olexandr-konovalov olexandr-konovalov added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Jan 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: added PRs introducing changes that have since been mentioned in the release notes topic: kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants