Skip to content

Add next_permutation and prev_permutation onto MutableTotalOrdVector<T>. #13761

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

Closed
wants to merge 1 commit into from
Closed

Conversation

exscape
Copy link
Contributor

@exscape exscape commented Apr 25, 2014

Okay, so first out, I'm no sure if this is okay as a pull request (without prior discussion or an RFC). However, considering that

Pull requests will be treated as "review requests"

and the fact that the community is supposed to be friendly to strangers and beginners, I figured not too much could go wrong; worst case, the pull request is not accepted, right? :-)
I looked at the existing RFCs, but all of them felt bigger in scope than simply adding a few function (causing no backwards incompatibility).

Anyway. This adds two methods to mutable vectors (MutableTotalOrdVector<T>): next_permutation() and prev_permutation().
They mutate the vector to the next/previous lexicographic permutation, in place.
The existing permutation functionality, ImmutableCloneableVector::permutations(), uses iterators and clones the entire vector each iteration, resulting in bad performance. For even a 10-element vector, that causes 10! = 3628800 calls to clone(), on the entire 10-element vector.
It also uses a swapping algorithm which does not generate lexicographic permutations; if you want lexicographical permutations, as far as I can tell, Rust currently cannot provide that.

So does Rust need this? I don't know. I wanted to use it, and realized it didn't exist, so I created it. Others may disagree about the necessity, of course.

Sample benchmarks:

test bench_iterated_permutations_one_iter           ... bench:       721 ns/iter (+/- 212)
test bench_lexicographic_permutations_one_iter      ... bench:        14 ns/iter (+/- 1)
test bench_iterated_permutations_bulk               ... bench:   2449743 ns/iter (+/- 328460)
test bench_lexicographic_permutations_bulk          ... bench:     69704 ns/iter (+/- 2235)

51x faster per single iteration (one permutation per benchmark iteration), 35x faster to loop through all 5040 permutations of range(0,7) per benchmark iteration.

One possible (style-ish) issue with this code is the return types: they return true if successful, and false if the vector was already at the first/last permutation. This makes looping rather easy, but I'm not sure if it's considered "pretty", so to speak. However, since iterators are much too expensive, at least if returning a cloned vector each iteration is the only way to make them work, I found no better way to solve this, and at least IMHO it looks fine.
Also, I'm not sure if the comments inside the actual functions is considered excessive or not, but that is of course easily changed, if that would be the case.

BTW, I have ran more tests than actually submitted; the full suite would be excessive to submit, but can be viewed here (please excuse the messy code).

@sfackler
Copy link
Member

Looks like there are some lines that are too long:

/home/travis/build/mozilla/rust/src/libstd/slice.rs:1844: line longer than 100 chars
/home/travis/build/mozilla/rust/src/libstd/slice.rs:1859: line longer than 100 chars

@exscape
Copy link
Contributor Author

exscape commented Apr 26, 2014

Oops, thanks. Should be fixed now.

@alexcrichton
Copy link
Member

Out of curiosity, is there a precedent for functions like this in other standard libraries?

@sfackler
Copy link
Member

@alexcrichton C++ has std::next_permutation and std::prev_permutation: http://www.cplusplus.com/reference/algorithm/next_permutation/, though they're a bit more generic since they're parameterized over iterators instead of just slices.

@exscape
Copy link
Contributor Author

exscape commented Apr 26, 2014

C++ has them in STL: next_permutation
Python has itertools.permutations():

import itertools
for iter in itertools.permutations([1,2,3]): print "{}".format(iter),
(1, 2, 3) (1, 3, 2) (2, 1, 3) (2, 3, 1) (3, 1, 2) (3, 2, 1)

Other than those two, I'm not sure, so I don't really know if it's typical or not.

Unlike ImmutableClonableVector::permutations() which returns an iterator,
cloning the entire array each iteration, these methods mutate the vector in-place.
For that reason, these methods are much faster; between 35-55 times faster,
depending on the benchmark. They also generate permutations in lexicographical order.
@exscape
Copy link
Contributor Author

exscape commented May 2, 2014

Sorry for the temporary screwup there (I assume updated pull requests are sent to those subscribed to a pull requests).
Anyway, I updated the code to allow for the methods to be used on ~[T] as well; thanks to dbaupp (via reddit).

exscape added a commit to exscape/Rust that referenced this pull request May 11, 2014
One of them (41-take2.rs) requires a still-not-accepted patch to the Rust
standard library; I'm not sure if it will ever be accepted, either.
rust-lang/rust#13761
@alexcrichton
Copy link
Member

Closing due to inactivity, but feel free to reopen with a rebase!

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