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

Provide a way for sets to zipper with more things #26595

Merged
merged 25 commits into from
Feb 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
a9abba9
Snapshot of fixing Set zippering
lydia-duncan Jan 10, 2025
cb73663
Check in a test locking in that the current implementation works
lydia-duncan Jan 10, 2025
a62e2ca
Make the leader actually parallel (oops)
lydia-duncan Jan 13, 2025
529c1c1
Make the chunk yielded reflect the number of elements we want each round
lydia-duncan Jan 13, 2025
09a4bca
Get the set/hashtable iterator to work with arrays as well
lydia-duncan Jan 13, 2025
75f5c49
Add three tests locking in the behavior when zippering with arrays
lydia-duncan Jan 13, 2025
7ee4eea
Add a version of setZipDifferentSize that reverses the order of the sets
lydia-duncan Jan 13, 2025
602bc55
Catch case where leader has more elements than follower, consistently
lydia-duncan Jan 16, 2025
a11a0ea
Try sending the number of slots in explicitly
lydia-duncan Jan 17, 2025
4ca6323
Try adding an alternate iterator to Set that takes a size argument
lydia-duncan Jan 22, 2025
dab6605
Cleanups
lydia-duncan Jan 22, 2025
375e1aa
Use size as the default instead of the internal information
lydia-duncan Jan 22, 2025
336dd2c
Add some more tests
lydia-duncan Jan 22, 2025
422a353
Merge branch 'main' into investigateSetZip
lydia-duncan Jan 23, 2025
2f33436
Add an unstable warning to one of the iterator overloads and a test o…
lydia-duncan Jan 23, 2025
1cdbbf7
Improve the error message for Set's part of the iterators
lydia-duncan Jan 23, 2025
24404fe
Update the unstable warning, per Ben's feedback
lydia-duncan Jan 24, 2025
719eb26
Revert "Get the set/hashtable iterator to work with arrays as well"
lydia-duncan Jan 29, 2025
f2d94ed
Drop the contents iterator and call the updated ChapelHashtable one p…
lydia-duncan Jan 29, 2025
c058c85
Update the tests
lydia-duncan Jan 29, 2025
c5b77fe
Remove another test I forgot about and the .bad for withRange1
lydia-duncan Jan 29, 2025
6438c6b
Use the issue number for the futures
lydia-duncan Jan 30, 2025
7267651
Merge branch 'main' into investigateSetZip
lydia-duncan Jan 30, 2025
c01c2d8
Simplify the implementation, at Brad's suggestion
lydia-duncan Jan 30, 2025
84e8447
Test updates for that change
lydia-duncan Jan 30, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion modules/internal/ChapelHashtable.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ module ChapelHashtable {
const minSizePerTask = dataParMinGranularity;

// We are simply slicing up the table here. Trying to do something
// more intelligent (like evenly dividing up the full slots, led
// more intelligent (like evenly dividing up the full slots), led
// to poor speed ups.

if debugAssocDataPar {
Expand Down Expand Up @@ -644,5 +644,34 @@ module ChapelHashtable {

rehash(newSize);
}

iter _evenSlots(param tag) where tag == iterKind.leader {
for i in (0..<this.tableNumFullSlots).these(tag) {
yield i;
}
}


iter _evenSlots(followThis, param tag) const ref
where tag == iterKind.follower {

var space = followThis(0);

var curNumFull = 0;

for i in 0..#(this.tableSize) {
if this.isSlotFull(i) {
if (curNumFull >= space.low) {
if (curNumFull <= space.high) {
yield this.table[i].key;
} else {
break;
}
}

curNumFull += 1;
}
}
}
}
}
7 changes: 4 additions & 3 deletions modules/standard/Set.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -564,16 +564,17 @@ module Set {
@chpldoc.nodoc
iter const these(param tag) where tag == iterKind.leader {
var space = 0..#_htb.tableSize;
for followThis in space.these(tag) {
for followThis in _htb._evenSlots(tag) {
yield followThis;
}
}
lydia-duncan marked this conversation as resolved.
Show resolved Hide resolved

@chpldoc.nodoc
iter const these(param tag, followThis) const ref
where tag == iterKind.follower {
foreach idx in followThis(0) do
if _htb.isSlotFull(idx) then yield _htb.table[idx].key;
foreach val in _htb._evenSlots(followThis, tag) {
yield val;
}
}

@chpldoc.nodoc
Expand Down
2 changes: 2 additions & 0 deletions test/library/standard/Set/these/setZipDifferentSize.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ proc test() {
var s2 = s1;
s2.add(9);

// We want to detect this case eventually, but it will be easier with
// with leader/follower improvements (see #11505)
forall (x, y) in zip(s1, s2) do
var tmp = x + y;

Expand Down
1 change: 0 additions & 1 deletion test/library/standard/Set/these/setZipDifferentSize.good
Original file line number Diff line number Diff line change
@@ -1 +0,0 @@
setZipDifferentSize.chpl:10: error: zippered iterations have non-equal lengths
16 changes: 16 additions & 0 deletions test/library/standard/Set/these/setZipDifferentSize2.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
use Set;

proc test() {
var s1: set(int);
for i in 1..8 do s1.add(i);

var s2 = s1;
s2.add(9);

forall (x, y) in zip(s2, s1) do
var tmp = x + y;

return;
}
test();

1 change: 1 addition & 0 deletions test/library/standard/Set/these/setZipDifferentSize2.good
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
setZipDifferentSize2.chpl:10: error: zippered iterations have non-equal lengths
2 changes: 2 additions & 0 deletions test/library/standard/Set/these/setZipDifferentSize2.skipif
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Follower checks are skipped when compiling with --fast
COMPOPTS <= --fast
21 changes: 21 additions & 0 deletions test/library/standard/Set/zippering/arrayShorter.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
use Set;

config const verbose = false;

var LocalSet= new set(int,parSafe = true);
LocalSet.add(1);
LocalSet.add(2);
LocalSet.add(3);
LocalSet.add(4);
LocalSet.add(5);

var A: [0..3] int;
writeln(A.size, " ", LocalSet.size);

// This should fail to zip, the set is longer than the array. But it doesn't,
// improvements to leader/follower (#11505) would make it easier to detect
forall (a,b) in zip(A,LocalSet) {
a=b;
if verbose then writeln(b);
}
writeln(A);
2 changes: 2 additions & 0 deletions test/library/standard/Set/zippering/arrayShorter.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
4 5
5 1 4 2
20 changes: 20 additions & 0 deletions test/library/standard/Set/zippering/arrayShorter2.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
use Set;

config const verbose = false;

var LocalSet= new set(int,parSafe = true);
LocalSet.add(1);
LocalSet.add(2);
LocalSet.add(3);
LocalSet.add(4);
LocalSet.add(5);

var A: [0..3] int;
writeln(A.size, " ", LocalSet.size);

// Expect this to fail to zip, the set is longer than the array
forall (b, a) in zip(LocalSet, A) {
a=b;
if verbose then writeln(b);
}
writeln(A);
2 changes: 2 additions & 0 deletions test/library/standard/Set/zippering/arrayShorter2.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
4 5
arrayShorter2.chpl:16: error: halt reached - size mismatch in zippered iteration (dimension 0)
13 changes: 13 additions & 0 deletions test/library/standard/Set/zippering/sameLength.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use Set;

var s1, s2: set(int);

for i in 1..10 { s1.add(i); s2.add(i+10); }

writeln("s1 = ", s1);
writeln("s2 = ", s2);

forall (i,j) in zip(s1, s2) do
writeln(i, " ", j);

writeln("all done");
13 changes: 13 additions & 0 deletions test/library/standard/Set/zippering/sameLength.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
all done
s1 = {5, 7, 8, 1, 4, 6, 2, 10, 9, 3}
s2 = {16, 20, 12, 17, 19, 13, 15, 11, 18, 14}
1 17
2 15
3 14
4 19
5 16
6 13
7 20
8 12
9 18
10 11
3 changes: 3 additions & 0 deletions test/library/standard/Set/zippering/sameLength.prediff
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#!/bin/sh
sort -n $1.exec.out.tmp > $1.prediff.tmp
mv $1.prediff.tmp $1.exec.out.tmp
17 changes: 17 additions & 0 deletions test/library/standard/Set/zippering/toArray.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
use Set;

var LocalSet= new set(int,parSafe = true);
LocalSet.add(1);
LocalSet.add(2);
LocalSet.add(3);
LocalSet.add(4);
LocalSet.add(5);

var A : [0..4] int;

writeln("A = ", A);
writeln("LocalSet = ", LocalSet);
forall (a,b) in zip (A,LocalSet.toArray()) {
a=b;
}
writeln("A = ", A);
3 changes: 3 additions & 0 deletions test/library/standard/Set/zippering/toArray.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
A = 0 0 0 0 0
LocalSet = {5, 1, 4, 2, 3}
A = 5 1 4 2 3
18 changes: 18 additions & 0 deletions test/library/standard/Set/zippering/withArray1.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
use Set;

config const verbose = false;

var LocalSet= new set(int,parSafe = true);
LocalSet.add(1);
LocalSet.add(2);
LocalSet.add(3);
LocalSet.add(4);
LocalSet.add(5);

var A : [0..4] int;
writeln(A.size, " ", LocalSet.size);
forall (a,b) in zip(A,LocalSet) {
a=b;
if verbose then writeln(b);
}
writeln(A);
2 changes: 2 additions & 0 deletions test/library/standard/Set/zippering/withArray1.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
5 5
5 1 4 2 3
18 changes: 18 additions & 0 deletions test/library/standard/Set/zippering/withArray2.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
use Set;

config const verbose = false;

var LocalSet= new set(int,parSafe = true);
LocalSet.add(1);
LocalSet.add(2);
LocalSet.add(3);
LocalSet.add(4);
LocalSet.add(5);

var A : [0..4] int;
writeln(A.size, " ", LocalSet.size);
forall (b, a) in zip(LocalSet, A) {
a=b;
if verbose then writeln(b);
}
writeln(A);
2 changes: 2 additions & 0 deletions test/library/standard/Set/zippering/withArray2.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
5 5
5 1 4 2 3
17 changes: 17 additions & 0 deletions test/library/standard/Set/zippering/withArrayLonger1.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
use Set;

config const verbose = false;

var LocalSet= new set(int,parSafe = true);
for i in 1..10 {
LocalSet.add(i);
}

var A : [0..9] int;
writeln(A.size, " ", LocalSet.size);
forall (a,b) in zip(A,LocalSet) {
a=b;
if verbose then writeln(b);
}
writeln(LocalSet);
writeln(A);
3 changes: 3 additions & 0 deletions test/library/standard/Set/zippering/withArrayLonger1.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
10 10
{5, 7, 8, 1, 4, 6, 2, 10, 9, 3}
5 7 8 1 4 6 2 10 9 3
14 changes: 14 additions & 0 deletions test/library/standard/Set/zippering/withRange1.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
use Set;

var s1: set(int);
var r1 = 1..10;

for i in 1..10 { s1.add(i); }

writeln("s1 = ", s1);
writeln("r1 = ", r1);

forall (i,j) in zip(r1, s1) do
writeln(i, " ", j);

writeln("all done");
13 changes: 13 additions & 0 deletions test/library/standard/Set/zippering/withRange1.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
all done
r1 = 1..10
s1 = {5, 7, 8, 1, 4, 6, 2, 10, 9, 3}
1 5
2 7
3 8
4 1
5 4
6 6
7 2
8 10
9 9
10 3
3 changes: 3 additions & 0 deletions test/library/standard/Set/zippering/withRange1.prediff
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#!/bin/sh
sort -n $1.exec.out.tmp > $1.prediff.tmp
mv $1.prediff.tmp $1.exec.out.tmp
14 changes: 14 additions & 0 deletions test/library/standard/Set/zippering/withRange2.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
use Set;

var s1: set(int);
var r1 = 1..10;

for i in 1..10 { s1.add(i); }

writeln("s1 = ", s1);
writeln("r1 = ", r1);

forall (i,j) in zip(s1, r1) do
writeln(i, " ", j);

writeln("all done");
13 changes: 13 additions & 0 deletions test/library/standard/Set/zippering/withRange2.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
all done
r1 = 1..10
s1 = {5, 7, 8, 1, 4, 6, 2, 10, 9, 3}
1 4
2 7
3 10
4 5
5 1
6 6
7 2
8 3
9 9
10 8
3 changes: 3 additions & 0 deletions test/library/standard/Set/zippering/withRange2.prediff
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#!/bin/sh
sort -n $1.exec.out.tmp > $1.prediff.tmp
mv $1.prediff.tmp $1.exec.out.tmp