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

Already on GitHub? Sign in to your account

List.combinations (#503) #513

Open
wants to merge 5 commits into
from

Conversation

Projects
None yet
3 participants
Member

c-cube commented Jan 20, 2014

Partial implementation for #503, compute combinations of elements of a list.

Member

UnixJunkie commented Feb 5, 2014

Is there some ocamldoc about what it does?

Member

UnixJunkie commented Feb 5, 2014

I would be very interested into having permutations in BatList also.

Member

UnixJunkie commented Feb 19, 2014

I am really interested into having combinations and permutations
in batteries.
BatList seems an interesting place to me.

Member

c-cube commented Feb 26, 2014

I just added permutations (although there is a lot of room for improvement performance-wise).

couldn't p1 and p2 have some better and more explicit names?

Member

c-cube commented Mar 2, 2014

Any thought on the algorithm used?

Member

UnixJunkie commented Mar 3, 2014

I'm OK with it, maybe you could have provided more unit tests though.

Owner

thelema commented Mar 9, 2014

This adds the first dependency of BatList on BatLazyList? It might be better to not have the dep in this direction. Because of this, maybe this code should go into BatLazyList? On the combinations algorithm, how about making a helper function subset: 'a list -> int -> 'a list that takes a list of size n and an integer < 2^n and returns the sublist of elements in positions that the binary expansion of the integer has a 1? Then combinations becomes fun l -> Enum.map (0--^(1 lsl (List.length l))) (subset l)

Member

c-cube commented Mar 9, 2014

Indeed, I chose BatLazyList because I thought it was more reasonable, but actually the combinators could return a BatEnum.t (future BatGen.t ;-)) that the caller could then convert into anything she wants. By the way, could LazyList.t or even BatSeq.t be structural types, using polymorphic variants (using a Nil` andCons` convention) to make those kinds of dependencies easier to deal with (or even just to interoperate with libraries, why not)?

I'm not sure I understand the argument about subset, would it work for lists longer than 63 elements? It looks like, to select elements in an arbitrary list, you'd rather use a BitSet.t.

Owner

thelema commented Mar 9, 2014

Combinations of more than 63 elements are already doomed; as there's no guarantee on the return order, we could spend eternity computing the first 2^63 subsets and report failure only if the user consumes all these. If this method is faster, it should be possible to use it to enumerate combinations of the first 63 elements and fall back to the current recursive method (or bitset) for further generation

Member

UnixJunkie commented Mar 10, 2014

The current version of BatList already depends on Enum.
So that would make Enum a natural candidate to replace LazyList.

Member

c-cube commented Mar 10, 2014

I'm not super enthusiastic about writing clone/count functions, so I might as well wait for the Gen version ;-)

I actually agree with @thelema argument about lists of more than 63 elements, it might work and be simpler.

Owner

thelema commented Mar 14, 2014

@simon It's perfectly reasonable to lazy the enum definition and not have
clone or count implemented. Just document it. maybe someone down the line
will have fun working this out.

On Mon, Mar 10, 2014 at 5:12 AM, Simon Cruanes notifications@github.comwrote:

I'm not super enthusiastic about writing clone/count functions, so I
might as well wait for the Gen version ;-)

I actually agree with @thelema https://github.com/thelema argument
about lists of more than 63 elements, it might work and be simpler.


Reply to this email directly or view it on GitHubhttps://github.com/ocaml-batteries-team/batteries-included/pull/513#issuecomment-37163773
.

@UnixJunkie UnixJunkie self-assigned this Apr 17, 2014

@UnixJunkie UnixJunkie removed their assignment Mar 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment