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.

Sign up for GitHub

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

Jump to bottom

added forceDepthFirst flag to closure and closureStar for testing purposes #237

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

added forceDepthFirst flag to closure and closureStar for testing purposes #237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c5f0f09
added forceDepthFirst flag to closure and closureStar for testing pur…
jurgenvinju Jan 12, 2024
afcd98a
added original methods closure() and closureStar back in for backward…
jurgenvinju Jan 12, 2024
849c655
fixed complex bug caused by wrong assumption
jurgenvinju Jan 12, 2024
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
22 changes: 20 additions & 2 deletions src/main/java/io/usethesource/vallang/IRelation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,15 @@ public default C compose(IRelation<C> that) {
* @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
* @throws UnsupportedOperationException when the receiver is not a binary relation
*/
public default C closure(boolean forceDepthFirst) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to break our public contract for this? How about still having the regular function in here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, yes we should have a default version.

Copy link
Member

@DavyLandman DavyLandman Jan 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about we give it an enum? or a way to say: auto vs DFS/BFS? so a user can say: hey, this is BFS, so let's go for that. Since even for big collections, there are graphs where BFS is better.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only for testing purposes. We can never give this to the rascal user anyway. And to be honest, there are dozens of algorithms we still have to try. It's quite possible that the distinction between DFS and bfs becomes a lot more vague.

if (!isBinary()) {
throw new UnsupportedOperationException("relation is not binary");
}
Expand All @@ -100,16 +109,25 @@ public default C closure() {
}

/**
* @return the transitive reflexive closure of a binary relation
* @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
* @throws UnsupportedOperationException when the receiver is not a binary relation
*/
public default C closureStar(boolean forceDepthFirst) {
IWriter<C> w = writer();

for (IValue val : carrier()) {
w.appendTuple(val, val);
}
w.appendAll(closure());
w.appendAll(closure(forceDepthFirst));

return w.done();
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/io/usethesource/vallang/impl/persistent/EmptySet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,12 @@ public ISet compose(IRelation<ISet> 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;
}

Expand Down
6 changes: 3 additions & 3 deletions src/main/java/io/usethesource/vallang/impl/persistent/ListRelation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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

Expand All @@ -60,7 +60,7 @@ public IList closureStar() {
reflex.insertTuple(e, e);
}

return closure().concat(reflex.done());
return closure(forceDepthFirst).concat(reflex.done());
}

}
33 changes: 15 additions & 18 deletions ...ain/java/io/usethesource/vallang/impl/persistent/PersistentHashIndexedBinaryRelation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ private static AbstractTypeBag calcTypeBag(SetMultimap<IValue, IValue> contents,
}

@Override
public ISet closure() {
public ISet closure(boolean forceDepthFirst) {
Type tupleType = getElementType();
assert tupleType.getArity() == 2;
Type keyType = tupleType.getFieldType(0);
Expand All @@ -554,7 +554,7 @@ public ISet closure() {
return this;
}

var result = computeClosure(content);
var result = computeClosure(content, forceDepthFirst);

final AbstractTypeBag keyTypeBag;
final AbstractTypeBag valTypeBag;
Expand All @@ -574,28 +574,25 @@ public ISet closure() {
}

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
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());
Expand All @@ -619,8 +616,8 @@ public ISet closureStar() {
return PersistentSetFactory.from(keyTypeBag, valTypeBag, result.freeze());
}

private static SetMultimap.Transient<IValue, IValue> computeClosure(final SetMultimap.Immutable<IValue, IValue> content) {
return content.size() > 256
private static SetMultimap.Transient<IValue, IValue> computeClosure(final SetMultimap.Immutable<IValue, IValue> content, boolean forceDepthFirst) {
return forceDepthFirst || content.size() > 256
? computeClosureDepthFirst(content)
: computeClosureBreadthFirst(content)
;
Expand Down
9 changes: 5 additions & 4 deletions src/test/java/io/usethesource/vallang/basic/ListRelationSmokeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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");
}
Expand All @@ -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");
Expand Down
9 changes: 5 additions & 4 deletions src/test/java/io/usethesource/vallang/basic/RelationSmokeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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");
}
Expand All @@ -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");
Expand Down
25 changes: 13 additions & 12 deletions src/test/java/io/usethesource/vallang/specification/IRelationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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++) {
Expand All @@ -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++) {
Expand All @@ -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++) {
Expand All @@ -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<ISet> r) {
Expand Down
Loading