Skip to content

Conversation

@davidavdav
Copy link
Contributor

@davidavdav davidavdav commented Jul 24, 2017

This is a tiny rewrite that makes nthperm!() a little faster, by about 15% or so. The speed results from replacing an effective k = k ÷ f by k = k - j * f, where j has been computed in an earlier %.

(Incidentally, this kind of code could benefit from a generic function divisor, remainder = integer_divide(num, denom))

Further some small cosmetic changes, and an unnecessary +1-1 removal for j.

@codecov-io
Copy link

codecov-io commented Jul 24, 2017

Codecov Report

Merging #55 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #55   +/-   ##
=======================================
  Coverage   96.96%   96.96%           
=======================================
  Files           6        6           
  Lines         594      594           
=======================================
  Hits          576      576           
  Misses         18       18
Impacted Files Coverage Δ
src/permutations.jl 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 84fe2fd...1d829b3. Read the comment docs.

@davidavdav
Copy link
Contributor Author

(Incidentally, this kind of code could benefit from a generic function divisor, remainder = integer_divide(num, denom))

Just found out there is such a function, divrem(), but this is slower than just doing a ÷ and a * as in the current patch.

k = k % f
f = div(f, n-i)
j = k ÷ f
k -= j * f
Copy link
Member

Choose a reason for hiding this comment

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

Yes, by k - div(k, f) * f == k % f

Copy link
Member

@mschauer mschauer left a comment

Choose a reason for hiding this comment

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

Not passing tests after resolving conflict

@mschauer
Copy link
Member

The issue is:
54bd3ee#diff-0da9215c8a020f2964e085699848213d

@mschauer mschauer merged commit f0f8cad into JuliaMath:master Oct 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants