From c5f0f0952391e77623cfd34175c1ab593b7ec6bc Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Fri, 12 Jan 2024 16:56:41 +0100 Subject: [PATCH 1/3] added forceDepthFirst flag to closure and closureStar for testing purposes --- .../io/usethesource/vallang/IRelation.java | 8 +++--- .../vallang/impl/persistent/EmptySet.java | 4 +-- .../vallang/impl/persistent/ListRelation.java | 6 ++--- .../PersistentHashIndexedBinaryRelation.java | 12 ++++----- .../vallang/basic/ListRelationSmokeTest.java | 9 ++++--- .../vallang/basic/RelationSmokeTest.java | 9 ++++--- .../vallang/specification/IRelationTests.java | 25 ++++++++++--------- 7 files changed, 39 insertions(+), 34 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/IRelation.java b/src/main/java/io/usethesource/vallang/IRelation.java index 834a37d90..77217a870 100644 --- a/src/main/java/io/usethesource/vallang/IRelation.java +++ b/src/main/java/io/usethesource/vallang/IRelation.java @@ -81,9 +81,10 @@ public default C compose(IRelation that) { /** * @return the transitive non-reflexive closure of a binary relation + * @param forceDepthFirst chooses the DFS algorithm over the BFS algorithm even though the size is small * @throws UnsupportedOperationException when the receiver is not a binary relation */ - public default C closure() { + public default C closure(boolean forceDepthFirst) { if (!isBinary()) { throw new UnsupportedOperationException("relation is not binary"); } @@ -101,15 +102,16 @@ public default C closure() { /** * @return the transitive reflexive closure of a binary relation + * @param forceDepthFirst chooses the DFS algorithm over the BFS algorithm even though the size is small * @throws UnsupportedOperationException when the receiver is not a binary relation */ - public default C closureStar() { + public default C closureStar(boolean forceDepthFirst) { IWriter w = writer(); for (IValue val : carrier()) { w.appendTuple(val, val); } - w.appendAll(closure()); + w.appendAll(closure(forceDepthFirst)); return w.done(); } diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/EmptySet.java b/src/main/java/io/usethesource/vallang/impl/persistent/EmptySet.java index d759e3335..cf5ac5eb0 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/EmptySet.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/EmptySet.java @@ -151,12 +151,12 @@ public ISet compose(IRelation that) { } @Override - public ISet closure() { + public ISet closure(boolean forceDepthFirst) { return EmptySet.this; } @Override - public ISet closureStar() { + public ISet closureStar(boolean forceDepthFirst) { return EmptySet.this; } diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/ListRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/ListRelation.java index d029d4fb9..547e7b855 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/ListRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/ListRelation.java @@ -21,7 +21,7 @@ public IList asContainer() { } @Override - public IList closure() { + public IList closure(boolean forceDepthFirst) { // will throw exception if not binary and reflexive list.getType().closure(); @@ -50,7 +50,7 @@ public IList closure() { } @Override - public IList closureStar() { + public IList closureStar(boolean forceDepthFirst) { list.getType().closure(); // an exception will have been thrown if the type is not acceptable @@ -60,7 +60,7 @@ public IList closureStar() { reflex.insertTuple(e, e); } - return closure().concat(reflex.done()); + return closure(forceDepthFirst).concat(reflex.done()); } } diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index 6d9b46278..bd6b4d795 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -543,7 +543,7 @@ private static AbstractTypeBag calcTypeBag(SetMultimap contents, } @Override - public ISet closure() { + public ISet closure(boolean forceDepthFirst) { Type tupleType = getElementType(); assert tupleType.getArity() == 2; Type keyType = tupleType.getFieldType(0); @@ -554,7 +554,7 @@ public ISet closure() { return this; } - var result = computeClosure(content); + var result = computeClosure(content, forceDepthFirst); final AbstractTypeBag keyTypeBag; final AbstractTypeBag valTypeBag; @@ -589,13 +589,13 @@ private boolean isConcreteValueType(Type keyType) { } @Override - public ISet closureStar() { + public ISet closureStar(boolean forceDepthFirst) { Type tupleType = getElementType(); assert tupleType.getArity() == 2; Type keyType = tupleType.getFieldType(0); Type valueType = tupleType.getFieldType(0); - var result = computeClosure(content); + var result = computeClosure(content, forceDepthFirst); for (var carrier: content.entrySet()) { result.__insert(carrier.getKey(), carrier.getKey()); @@ -619,8 +619,8 @@ public ISet closureStar() { return PersistentSetFactory.from(keyTypeBag, valTypeBag, result.freeze()); } - private static SetMultimap.Transient computeClosure(final SetMultimap.Immutable content) { - return content.size() > 256 + private static SetMultimap.Transient computeClosure(final SetMultimap.Immutable content, boolean forceDepthFirst) { + return forceDepthFirst || content.size() > 256 ? computeClosureDepthFirst(content) : computeClosureBreadthFirst(content) ; diff --git a/src/test/java/io/usethesource/vallang/basic/ListRelationSmokeTest.java b/src/test/java/io/usethesource/vallang/basic/ListRelationSmokeTest.java index 78914fdbc..1de1a67ad 100644 --- a/src/test/java/io/usethesource/vallang/basic/ListRelationSmokeTest.java +++ b/src/test/java/io/usethesource/vallang/basic/ListRelationSmokeTest.java @@ -19,6 +19,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; +import io.usethesource.vallang.IBool; import io.usethesource.vallang.IList; import io.usethesource.vallang.IListWriter; import io.usethesource.vallang.ITuple; @@ -182,9 +183,9 @@ public void testProductIList(IValueFactory vf) { } // @ParameterizedTest @ArgumentsSource(ValueProvider.class) - public void xtestClosure(IValueFactory vf) { + public void xtestClosure(IValueFactory vf, IBool forceDepthFirst) { try { - if (!integerListRelation(vf).asRelation().closure().equals(integerListRelation(vf))) { + if (!integerListRelation(vf).asRelation().closure(forceDepthFirst.getValue()).equals(integerListRelation(vf))) { fail("closure adds extra tuples?"); } } catch (FactTypeUseException e) { @@ -195,7 +196,7 @@ public void xtestClosure(IValueFactory vf) { ITuple t1 = vf.tuple(integers(vf)[0], integers(vf)[1]); IList rel = vf.list(t1); - rel.asRelation().closure(); + rel.asRelation().closure(forceDepthFirst.getValue()); } catch (FactTypeUseException e) { fail("reflexivity with subtyping is allowed"); } @@ -209,7 +210,7 @@ public void xtestClosure(IValueFactory vf) { ITuple t6 = vf.tuple(integers(vf)[0], integers(vf)[3]); IList test = vf.list(t1, t2, t3); - IList closed = test.asRelation().closure(); + IList closed = test.asRelation().closure(forceDepthFirst.getValue()); if (closed.asRelation().arity() != test.asRelation().arity()) { fail("closure should produce relations of same arity"); diff --git a/src/test/java/io/usethesource/vallang/basic/RelationSmokeTest.java b/src/test/java/io/usethesource/vallang/basic/RelationSmokeTest.java index 43896034f..942afda7c 100644 --- a/src/test/java/io/usethesource/vallang/basic/RelationSmokeTest.java +++ b/src/test/java/io/usethesource/vallang/basic/RelationSmokeTest.java @@ -20,6 +20,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; +import io.usethesource.vallang.IBool; import io.usethesource.vallang.ISet; import io.usethesource.vallang.ISetWriter; import io.usethesource.vallang.ITuple; @@ -183,9 +184,9 @@ public void testProductISet(IValueFactory vf) { } @ParameterizedTest @ArgumentsSource(ValueProvider.class) - public void testClosure(IValueFactory vf) { + public void testClosure(IValueFactory vf, IBool forceDepthFirst) { try { - if (!integerRelation(vf).asRelation().closure().equals(integerRelation(vf))) { + if (!integerRelation(vf).asRelation().closure(forceDepthFirst.getValue()).equals(integerRelation(vf))) { fail("closure adds extra tuples?"); } } catch (FactTypeUseException e) { @@ -194,7 +195,7 @@ public void testClosure(IValueFactory vf) { try { ISet rel = vf.set(); - rel.asRelation().closure(); + rel.asRelation().closure(forceDepthFirst.getValue()); } catch (FactTypeUseException e) { fail("reflexivity with subtyping is allowed"); } @@ -208,7 +209,7 @@ public void testClosure(IValueFactory vf) { ITuple t6 = vf.tuple(integers(vf)[0], integers(vf)[3]); ISet test = vf.set(t1, t2, t3); - ISet closed = test.asRelation().closure(); + ISet closed = test.asRelation().closure(forceDepthFirst.getValue()); if (closed.asRelation().arity() != test.asRelation().arity()) { fail("closure should produce relations of same arity"); diff --git a/src/test/java/io/usethesource/vallang/specification/IRelationTests.java b/src/test/java/io/usethesource/vallang/specification/IRelationTests.java index c15e3b3ea..959bd490c 100644 --- a/src/test/java/io/usethesource/vallang/specification/IRelationTests.java +++ b/src/test/java/io/usethesource/vallang/specification/IRelationTests.java @@ -11,6 +11,7 @@ import io.usethesource.vallang.ExpectedType; import io.usethesource.vallang.GivenValue; +import io.usethesource.vallang.IBool; import io.usethesource.vallang.IList; import io.usethesource.vallang.IRelation; import io.usethesource.vallang.ISet; @@ -42,22 +43,22 @@ public void listRelationProjectOrder(IValueFactory vf, @ExpectedType("lrel[int,i @ParameterizedTest @ArgumentsSource(ValueProvider.class) public void transReflexiveClosure( @GivenValue("{<1,2>, <2,3>, <3,4>}") ISet src, - @GivenValue("{<1,2>, <2,3>, <3,4>, <1, 3>, <2, 4>, <1, 4>, <1, 1>, <2, 2>, <3, 3>, <4, 4>}") ISet result) { - assertEquals(src.asRelation().closureStar(), result); + @GivenValue("{<1,2>, <2,3>, <3,4>, <1, 3>, <2, 4>, <1, 4>, <1, 1>, <2, 2>, <3, 3>, <4, 4>}") ISet result, IBool forceDepthFirst) { + assertEquals(src.asRelation().closureStar(forceDepthFirst.getValue()), result); } @ParameterizedTest @ArgumentsSource(ValueProvider.class) - public void transClosure(@ExpectedType("rel[int,int]") ISet src) { - assertEquals(src.asRelation().closure().intersect(src), src); + public void transClosure(@ExpectedType("rel[int,int]") ISet src, IBool forceDepthFirst) { + assertEquals(src.asRelation().closure(forceDepthFirst.getValue()).intersect(src), src); } @ParameterizedTest @ArgumentsSource(ValueProvider.class) - public void transClosureLocs(@ExpectedType("rel[loc,loc]") ISet src) { - assertEquals(src.asRelation().closure().intersect(src), src); + public void transClosureLocs(@ExpectedType("rel[loc,loc]") ISet src, IBool forceDepthFirst) { + assertEquals(src.asRelation().closure(forceDepthFirst.getValue()).intersect(src), src); } @ParameterizedTest @ArgumentsSource(ValueProvider.class) - public void deepClosure(IValueFactory vf) { + public void deepClosure(IValueFactory vf, IBool forceDepthFirst) { IValue prev = vf.integer(0); var rBuilder = vf.setWriter(); for (int i=1; i < 100; i++) { @@ -67,11 +68,11 @@ public void deepClosure(IValueFactory vf) { } var r = rBuilder.done().asRelation(); - assertEquals(slowClosure(vf, r),r.closure(),() -> "Failed with input: " + r.toString()); + assertEquals(slowClosure(vf, r),r.closure(forceDepthFirst.getValue()),() -> "Failed with input: " + r.toString()); } @ParameterizedTest @ArgumentsSource(ValueProvider.class) - public void broadClosure(IValueFactory vf) { + public void broadClosure(IValueFactory vf, IBool forceDepthFirst) { IValue prev = vf.integer(0); var rBuilder = vf.setWriter(); for (int i=1; i < 100; i++) { @@ -83,11 +84,11 @@ public void broadClosure(IValueFactory vf) { } var r = rBuilder.done().asRelation(); - assertEquals(slowClosure(vf, r),r.closure(),() -> "Failed with input: " + r.toString()); + assertEquals(slowClosure(vf, r),r.closure(forceDepthFirst.getValue()),() -> "Failed with input: " + r.toString()); } @ParameterizedTest @ArgumentsSource(ValueProvider.class) - public void severalDeep(IValueFactory vf) { + public void severalDeep(IValueFactory vf, IBool forceDepthFirst) { IValue prev = vf.integer(0); var rBuilder = vf.setWriter(); for (int i=1; i < 100; i++) { @@ -108,7 +109,7 @@ public void severalDeep(IValueFactory vf) { } var r = rBuilder.done().asRelation(); - assertEquals(slowClosure(vf, r),r.closure(),() -> "Failed with input: " + r.toString()); + assertEquals(slowClosure(vf, r),r.closure(forceDepthFirst.getValue()),() -> "Failed with input: " + r.toString()); } private ISet slowClosure(IValueFactory vf, IRelation r) { From afcd98a29dbf960e590aace324bfc11acf91f736 Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Fri, 12 Jan 2024 17:29:38 +0100 Subject: [PATCH 2/3] added original methods closure() and closureStar back in for backward compatibility --- .../java/io/usethesource/vallang/IRelation.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/main/java/io/usethesource/vallang/IRelation.java b/src/main/java/io/usethesource/vallang/IRelation.java index 77217a870..e90033aea 100644 --- a/src/main/java/io/usethesource/vallang/IRelation.java +++ b/src/main/java/io/usethesource/vallang/IRelation.java @@ -79,6 +79,14 @@ public default C compose(IRelation that) { return w.done(); } + /** + * @return the transitive non-reflexive closure of a binary relation + * @throws UnsupportedOperationException when the receiver is not a binary relation + */ + public default C closure() { + return closure(false); + } + /** * @return the transitive non-reflexive closure of a binary relation * @param forceDepthFirst chooses the DFS algorithm over the BFS algorithm even though the size is small @@ -100,6 +108,14 @@ public default C closure(boolean forceDepthFirst) { return next; } + /** + * @return the transitive non-reflexive closure of a binary relation + * @throws UnsupportedOperationException when the receiver is not a binary relation + */ + public default C closureStar() { + return closureStar(false); + } + /** * @return the transitive reflexive closure of a binary relation * @param forceDepthFirst chooses the DFS algorithm over the BFS algorithm even though the size is small From 849c655b85c9bb9537c36584988ea11223e074c8 Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Fri, 12 Jan 2024 17:49:31 +0100 Subject: [PATCH 3/3] fixed complex bug caused by wrong assumption --- .../PersistentHashIndexedBinaryRelation.java | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java index bd6b4d795..8cd223ee5 100644 --- a/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java +++ b/src/main/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java @@ -574,18 +574,15 @@ public ISet closure(boolean forceDepthFirst) { } private boolean isConcreteValueType(Type keyType) { - if (keyType.isList() || keyType.isSet() || keyType.isMap()) { - return false; - } - - if (keyType.isAbstractData() && keyType.isParameterized()) { - return false; // could have abstract type parameters that can be different for different tuples - } - - Type voidType = TypeFactory.getInstance().voidType(); - - // this is a quick check for int, real, rat, loc, str (not num, not node, etc) - return keyType.glb(voidType) == voidType; + return keyType.isSourceLocation() + || keyType.isInteger() + || keyType.isRational() + || keyType.isReal() + || keyType.isDateTime() + || (keyType.isAbstractData() && !keyType.isParameterized()) + || keyType.isString() + || keyType.isBool() + ; } @Override