Implement std.algorithm.cartesianProduct #856

Merged
merged 6 commits into from Feb 4, 2013

Conversation

Projects
None yet
4 participants
Member

quickfur commented Oct 10, 2012

As discussed in the forum. This implementation supports infinite ranges as long as they satisfy certain range requirements.

Member

quickfur commented Oct 10, 2012

Yikes! Looks like the autotester is failing on issue 8556 (cf. 8790). :-(

Member

quickfur commented Oct 13, 2012

Fixed coding style, rebased.

Owner

andralex commented Oct 15, 2012

Quick comment - I think we should sleep on getting the infinite forward ranges implementation right; if we commit this the risk is to discourage more general implementations.

The idea for merge on two infinite forward ranges is as follows. Imagine we show the ranges in a plane with r1 horizontally and r2 vertically. Then we first walk the square of side 1. There's only one: tuple(r1[0], r2[0]). Then we walk the square of side 2: (0, 1), (1, 0), and (1, 1). Then we walk the square of side 3: (0, 2), (1, 2), (2, 0), (2, 1), (2, 2). Note that how at each step one index is constant and the other either grows by exactly 1 or it gets reset to 0, thus making the algorithm implementable with forward ranges. The jump from the corner (1, 1) to (0, 2) is a bit special: one range is advanced and the other is reset.

Member

quickfur commented Oct 15, 2012

That's an excellent idea! So basically instead of diagonals, we alternate between vertical and horizontal line segments of increasing lengths, which meet at the diagonal. I'll think about how to implement this.

@denis-sh denis-sh commented on an outdated diff Oct 30, 2012

std/algorithm.d
+
+ assert(equal(map!"[a[0],a[1]]"(NB.take(10)), [[0, 1], [0, 2], [0, 3],
+ [1, 1], [1, 2], [1, 3], [2, 1], [2, 2], [2, 3], [3, 1]]));
+
+ // General finite range case
+ auto C = [ 4, 5, 6 ];
+ auto BC = cartesianProduct(B, C);
+
+ foreach (n; [[1, 4], [2, 4], [3, 4], [1, 5], [2, 5], [3, 5], [1, 6],
+ [2, 6], [3, 6]])
+ {
+ assert(canFind(BC, tuple(n[0], n[1])));
+ }
+}
+
+// vim:set sw=4 ts=4 expandtab:
@denis-sh

denis-sh Oct 30, 2012

Contributor

Probably unrelated line. )

Owner

andralex commented Nov 3, 2012

This is good work. I think it's important that we fix the elusive compiler bug to allow this and many other instances to work. cc @9rnsr

Member

quickfur commented Nov 5, 2012

Rebased and updated to Andrei's new schedule that only requires forward ranges for the two infinite ranges case. Unittests still do not compile due to the bugs as mentioned in the forum, but I thought I should at least put the code up for review first.

Member

quickfur commented Dec 14, 2012

Just for reference, here's the cause of the current autotester failures:
http://d.puremagic.com/issues/show_bug.cgi?id=8542

Member

quickfur commented Dec 20, 2012

Rebased to prevent from going stale. Since issue 8542 is still open, the autotester will probably still fail, though. :-(

Contributor

Poita commented Dec 27, 2012

Is there anything you can do to workaround the bug? It would be nice to get this in if the bug is going to take a while to fix.

Member

quickfur commented Dec 30, 2012

Unfortunately I'd have to disable a lot of unittests for that to work. It might be possible to split up the unittests, though. I'll give it a shot when I get home (I'm travelling currently).

Member

quickfur commented Jan 1, 2013

Rebased, and rewrote unittests to avoid issue 8542. The original unittests are version'd out for future reference, in case we wish to run the old tests again after issue 8542 is fixed.

Member

quickfur commented Jan 5, 2013

Is std.algorithm getting too big for some of the autotester machines?

http://d.puremagic.com/test-results/pull.ghtml?runid=440874

quickfur added some commits Oct 10, 2012

@quickfur quickfur Preliminary implementation of cartesianProduct.
For the case of two infinite ranges, use Andrei's schedule of traversing
right and bottom edges of an increasing square area over the infinite
table of combinations. This allows us to only need to traverse each
range in one direction, so only forward ranges are needed.
fcb95e6
@quickfur quickfur Add cartesianProduct to cheatsheet. 4ac5350
@quickfur quickfur Replace unittests to avoid issue 8542. 468f7e1
@quickfur quickfur Improve unittests.
Make unittests independent of exact order of pairs produced by
cartesianProduct.

Add unittest for cartesian product of two finite ranges.
4c5f3d2
@quickfur quickfur Delete unrelated line. 31fdd79
@quickfur quickfur Fix -property errors. 310216e
Member

quickfur commented Feb 4, 2013

Rebased to fix merge conflict.

Owner

andralex commented Feb 4, 2013

Looks like we're good to go! Any tests on composing cartesianProduct with itself for taking the product of any number of ranges?

Owner

andralex commented Feb 4, 2013

I'll merge this now anyhow, we can think of adding variadics later.

@andralex andralex added a commit that referenced this pull request Feb 4, 2013

@andralex andralex Merge pull request #856 from quickfur/cartprod
Implement std.algorithm.cartesianProduct
8abbbb1

@andralex andralex merged commit 8abbbb1 into dlang:master Feb 4, 2013

1 check passed

default Pass: 10
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment