Skip to content

Commit c9dd898

Browse files
committed
Address review comments
1 parent f7813b3 commit c9dd898

File tree

9 files changed

+93
-68
lines changed

9 files changed

+93
-68
lines changed

core/src/main/java/ai/timefold/solver/core/impl/move/streams/dataset/bi/BiMapBiDataStream.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ final class BiMapBiDataStream<Solution_, A, B, NewA, NewB>
1919
private final BiDataMapper<Solution_, A, B, NewB> mappingFunctionB;
2020
private @Nullable AftBridgeBiDataStream<Solution_, NewA, NewB> aftStream;
2121

22-
public BiMapBiDataStream(DataStreamFactory<Solution_> constraintFactory, AbstractBiDataStream<Solution_, A, B> parent,
22+
public BiMapBiDataStream(DataStreamFactory<Solution_> dataStreamFactory, AbstractBiDataStream<Solution_, A, B> parent,
2323
BiDataMapper<Solution_, A, B, NewA> mappingFunctionA, BiDataMapper<Solution_, A, B, NewB> mappingFunctionB) {
24-
super(constraintFactory, parent);
24+
super(dataStreamFactory, parent);
2525
this.mappingFunctionA = mappingFunctionA;
2626
this.mappingFunctionB = mappingFunctionB;
2727
}

core/src/main/java/ai/timefold/solver/core/impl/move/streams/dataset/bi/UniMapBiDataStream.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,15 @@
1313
import org.jspecify.annotations.Nullable;
1414

1515
@NullMarked
16-
final class UniMapBiDataStream<Solution_, A, B, NewA, NewB>
16+
final class UniMapBiDataStream<Solution_, A, B, NewA>
1717
extends AbstractUniDataStream<Solution_, NewA> {
1818

1919
private final BiDataMapper<Solution_, A, B, NewA> mappingFunction;
2020
private @Nullable AftBridgeUniDataStream<Solution_, NewA> aftStream;
2121

22-
public UniMapBiDataStream(DataStreamFactory<Solution_> constraintFactory, AbstractBiDataStream<Solution_, A, B> parent,
22+
public UniMapBiDataStream(DataStreamFactory<Solution_> dataStreamFactory, AbstractBiDataStream<Solution_, A, B> parent,
2323
BiDataMapper<Solution_, A, B, NewA> mappingFunction) {
24-
super(constraintFactory, parent);
24+
super(dataStreamFactory, parent);
2525
this.mappingFunction = mappingFunction;
2626
}
2727

@@ -51,7 +51,7 @@ public boolean equals(Object object) {
5151
return true;
5252
if (object == null || getClass() != object.getClass())
5353
return false;
54-
UniMapBiDataStream<?, ?, ?, ?, ?> that = (UniMapBiDataStream<?, ?, ?, ?, ?>) object;
54+
UniMapBiDataStream<?, ?, ?, ?> that = (UniMapBiDataStream<?, ?, ?, ?>) object;
5555
return Objects.equals(parent, that.parent) &&
5656
Objects.equals(mappingFunction, that.mappingFunction);
5757
}
@@ -63,7 +63,7 @@ public int hashCode() {
6363

6464
@Override
6565
public String toString() {
66-
return "BiMap()";
66+
return "UniMap()";
6767
}
6868

6969
}

core/src/main/java/ai/timefold/solver/core/impl/move/streams/dataset/common/AbstractDataStream.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,13 @@ protected boolean guaranteesDistinct() {
4242
// Node creation
4343
// ************************************************************************
4444

45-
public void collectActiveDataStreams(Set<AbstractDataStream<Solution_>> constraintStreamSet) {
45+
public void collectActiveDataStreams(Set<AbstractDataStream<Solution_>> dataStreamSet) {
4646
if (parent == null) { // Maybe a join/ifExists/forEach forgot to override this?
47-
throw new IllegalStateException("Impossible state: the stream (" + this + ") does not have a parent.");
47+
throw new IllegalStateException("Impossible state: the stream (%s) does not have a parent."
48+
.formatted(this));
4849
}
49-
parent.collectActiveDataStreams(constraintStreamSet);
50-
constraintStreamSet.add(this);
50+
parent.collectActiveDataStreams(dataStreamSet);
51+
dataStreamSet.add(this);
5152
}
5253

5354
/**

core/src/main/java/ai/timefold/solver/core/impl/move/streams/dataset/uni/AbstractUniDataStream.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import java.util.Objects;
66
import java.util.function.Function;
77

8-
import ai.timefold.solver.core.api.score.stream.uni.UniConstraintStream;
98
import ai.timefold.solver.core.impl.bavet.common.GroupNodeConstructor;
109
import ai.timefold.solver.core.impl.bavet.common.tuple.UniTuple;
1110
import ai.timefold.solver.core.impl.bavet.uni.Group1Mapping0CollectorUniNode;
@@ -100,13 +99,13 @@ private <B> UniDataStream<Solution_, A> ifExistsOrNot(boolean shouldExist, UniDa
10099
}
101100

102101
/**
103-
* Convert the {@link UniConstraintStream} to a different {@link UniConstraintStream},
102+
* Convert the {@link UniDataStream} to a different {@link UniDataStream},
104103
* containing the set of tuples resulting from applying the group key mapping function
105104
* on all tuples of the original stream.
106105
* Neither tuple of the new stream {@link Objects#equals(Object, Object)} any other.
107106
*
108107
* @param groupKeyMapping mapping function to convert each element in the stream to a different element
109-
* @param <GroupKey_> the type of a fact in the destination {@link UniConstraintStream}'s tuple;
108+
* @param <GroupKey_> the type of a fact in the destination {@link UniDataStream}'s tuple;
110109
* must honor {@link Object#hashCode() the general contract of hashCode}.
111110
*/
112111
protected <GroupKey_> AbstractUniDataStream<Solution_, GroupKey_> groupBy(Function<A, GroupKey_> groupKeyMapping) {

core/src/main/java/ai/timefold/solver/core/impl/move/streams/dataset/uni/BiMapUniDataStream.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ final class BiMapUniDataStream<Solution_, A, NewA, NewB>
2020
private final UniDataMapper<Solution_, A, NewB> mappingFunctionB;
2121
private @Nullable AftBridgeBiDataStream<Solution_, NewA, NewB> aftStream;
2222

23-
public BiMapUniDataStream(DataStreamFactory<Solution_> constraintFactory, AbstractUniDataStream<Solution_, A> parent,
23+
public BiMapUniDataStream(DataStreamFactory<Solution_> dataStreamFactory, AbstractUniDataStream<Solution_, A> parent,
2424
UniDataMapper<Solution_, A, NewA> mappingFunctionA, UniDataMapper<Solution_, A, NewB> mappingFunctionB) {
25-
super(constraintFactory, parent);
25+
super(dataStreamFactory, parent);
2626
this.mappingFunctionA = mappingFunctionA;
2727
this.mappingFunctionB = mappingFunctionB;
2828
}

core/src/main/java/ai/timefold/solver/core/impl/move/streams/dataset/uni/UniMapUniDataStream.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ final class UniMapUniDataStream<Solution_, A, NewA>
1818
private final UniDataMapper<Solution_, A, NewA> mappingFunction;
1919
private @Nullable AftBridgeUniDataStream<Solution_, NewA> aftStream;
2020

21-
public UniMapUniDataStream(DataStreamFactory<Solution_> constraintFactory, AbstractUniDataStream<Solution_, A> parent,
21+
public UniMapUniDataStream(DataStreamFactory<Solution_> dataStreamFactory, AbstractUniDataStream<Solution_, A> parent,
2222
UniDataMapper<Solution_, A, NewA> mappingFunction) {
23-
super(constraintFactory, parent);
23+
super(dataStreamFactory, parent);
2424
this.mappingFunction = mappingFunction;
2525
}
2626

core/src/main/java/ai/timefold/solver/core/impl/move/streams/maybeapi/generic/move/ListChangeMove.java

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -174,14 +174,12 @@ public int getDestinationIndex() {
174174

175175
@Override
176176
public boolean equals(Object o) {
177-
if (this == o)
178-
return true;
179-
if (!(o instanceof ListChangeMove<?, ?, ?> that))
180-
return false;
181-
return sourceIndex == that.sourceIndex && destinationIndex == that.destinationIndex
182-
&& Objects.equals(variableMetaModel, that.variableMetaModel)
183-
&& Objects.equals(sourceEntity, that.sourceEntity)
184-
&& Objects.equals(destinationEntity, that.destinationEntity);
177+
return o instanceof ListChangeMove<?, ?, ?> other
178+
&& Objects.equals(variableMetaModel, other.variableMetaModel)
179+
&& Objects.equals(sourceEntity, other.sourceEntity)
180+
&& sourceIndex == other.sourceIndex
181+
&& Objects.equals(destinationEntity, other.destinationEntity)
182+
&& destinationIndex == other.destinationIndex;
185183
}
186184

187185
@Override

core/src/main/java/ai/timefold/solver/core/impl/move/streams/maybeapi/generic/provider/ListChangeMoveProvider.java

Lines changed: 65 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,35 @@
1414
import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningListVariableMetaModel;
1515
import ai.timefold.solver.core.preview.api.domain.metamodel.PositionInList;
1616
import ai.timefold.solver.core.preview.api.domain.metamodel.UnassignedElement;
17+
import ai.timefold.solver.core.preview.api.move.SolutionView;
1718

1819
import org.jspecify.annotations.NullMarked;
1920

21+
/**
22+
* For each unassigned value, creates a move to assign it to some position of some list variable.
23+
* For each assigned value that is not pinned, creates:
24+
*
25+
* <ul>
26+
* <li>A move to unassign it.</li>
27+
* <li>A move to reassign it to another position if assigned.</li>
28+
* </ul>
29+
*
30+
* To assign or reassign a value, creates:
31+
*
32+
* <ul>
33+
* <li>A move for every unpinned value in every entity's list variable to assign the value before that position.</li>
34+
* <li>A move for every entity to assign it to the last position in the list variable.</li>
35+
* </ul>
36+
*
37+
* This is a generic move provider that works with any list variable;
38+
* user-defined change move providers needn't be this complex, as they understand the specifics of the domain.
39+
*/
2040
@NullMarked
2141
public class ListChangeMoveProvider<Solution_, Entity_, Value_>
2242
implements MoveProvider<Solution_> {
2343

2444
private final PlanningListVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel;
2545
private final BiDataFilter<Solution_, Entity_, Value_> isValueInListFilter;
26-
private final BiDataFilter<Solution_, Value_, ElementPosition> validChangeFilter;
2746

2847
public ListChangeMoveProvider(PlanningListVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel) {
2948
this.variableMetaModel = Objects.requireNonNull(variableMetaModel);
@@ -36,51 +55,23 @@ public ListChangeMoveProvider(PlanningListVariableMetaModel<Solution_, Entity_,
3655
}
3756
return solution.isValueInRange(variableMetaModel, entity, value);
3857
};
39-
this.validChangeFilter = (solutionView, value, targetPosition) -> {
40-
var currentPosition = solutionView.getPositionOf(variableMetaModel, value);
41-
if (currentPosition.equals(targetPosition)) { // No change needed.
42-
return false;
43-
}
44-
if (currentPosition instanceof UnassignedElement) {
45-
// Only assign the value if the target entity will accept it.
46-
var targetPositionInList = targetPosition.ensureAssigned();
47-
return solutionView.isValueInRange(variableMetaModel, targetPositionInList.entity(), value);
48-
} else {
49-
if (!(targetPosition instanceof PositionInList targetPositionInList)) { // Unassigning a value.
50-
return true;
51-
}
52-
var currentPositionInList = currentPosition.ensureAssigned();
53-
if (currentPositionInList.entity() == targetPositionInList.entity()) {
54-
var valueCount = solutionView.countValues(variableMetaModel, currentPositionInList.entity());
55-
if (valueCount == 1) {
56-
return false; // The value is already in the list, and it is the only one.
57-
} else if (targetPositionInList.index() == valueCount) {
58-
// The value is already in the list, and we are trying to move it past the end of the list.
59-
return false;
60-
} else { // Same list, same position; ignore.
61-
return currentPositionInList.index() != targetPositionInList.index();
62-
}
63-
} else { // We can move freely between entities, assuming the target entity accepts the value.
64-
return solutionView.isValueInRange(variableMetaModel, targetPositionInList.entity(), value);
65-
}
66-
}
67-
};
6858
}
6959

7060
@Override
7161
public MoveProducer<Solution_> apply(MoveStreamFactory<Solution_> moveStreamFactory) {
72-
// For each unassigned value, we need to create a move to assign it to some position of some list variable.
73-
// For each assigned value that is not pinned, we need to create:
74-
// - A move to unassign it.
75-
// - A move to reassign it to another position if assigned.
76-
// To assign or reassign a value, we need to create:
77-
// - A move for every unpinned value in every entity's list variable to assign the value before that position.
78-
// - A move for every entity to assign it to the last position in the list variable.
62+
// Stream with unpinned entities;
63+
// includes null if the variable allows unassigned values.
7964
var unpinnedEntities =
8065
moveStreamFactory.enumerate(variableMetaModel.entity().type(), variableMetaModel.allowsUnassignedValues());
66+
// Stream with unpinned values, which are assigned to any list variable;
67+
// always includes null so that we can later create a position at the end of the list,
68+
// i.e. with no value after it.
8169
var unpinnedValues = moveStreamFactory.enumerate(variableMetaModel.type(), true)
8270
.filter((solutionView, value) -> value == null
8371
|| solutionView.getPositionOf(variableMetaModel, value) instanceof PositionInList);
72+
// Joins the two previous streams to create pairs of (entity, value),
73+
// eliminating values which do not match that entity's value range.
74+
// It maps these pairs to expected target positions in that entity's list variable.
8475
var entityValuePairs = unpinnedEntities.join(unpinnedValues, DataJoiners.filtering(isValueInListFilter))
8576
.map((solutionView, entity, value) -> {
8677
if (entity == null) { // Null entity means we need to unassign the value.
@@ -94,8 +85,12 @@ public MoveProducer<Solution_> apply(MoveStreamFactory<Solution_> moveStreamFact
9485
}
9586
})
9687
.distinct();
88+
// Finally the stream of these positions is joined with the stream of all existing values,
89+
// filtering out those which would not result in a valid move.
9790
var dataStream = moveStreamFactory.enumerate(variableMetaModel.type(), false)
98-
.join(entityValuePairs, DataJoiners.filtering(validChangeFilter));
91+
.join(entityValuePairs, DataJoiners.filtering(this::isValidChange));
92+
// When picking from this stream, we decide what kind of move we need to create,
93+
// based on whether the value is assigned or unassigned.
9994
return moveStreamFactory.pick(dataStream)
10095
.asMove((solutionView, value, targetPosition) -> {
10196
var currentPosition = solutionView.getPositionOf(variableMetaModel, Objects.requireNonNull(value));
@@ -115,4 +110,36 @@ public MoveProducer<Solution_> apply(MoveStreamFactory<Solution_> moveStreamFact
115110
});
116111
}
117112

113+
private boolean isValidChange(SolutionView<Solution_> solutionView, Value_ value, ElementPosition targetPosition) {
114+
var currentPosition = solutionView.getPositionOf(variableMetaModel, value);
115+
if (currentPosition.equals(targetPosition)) { // No change needed.
116+
return false;
117+
}
118+
119+
if (currentPosition instanceof UnassignedElement) { // Only assign the value if the target entity will accept it.
120+
var targetPositionInList = targetPosition.ensureAssigned();
121+
return solutionView.isValueInRange(variableMetaModel, targetPositionInList.entity(), value);
122+
}
123+
124+
if (!(targetPosition instanceof PositionInList targetPositionInList)) { // Unassigning a value.
125+
return true;
126+
}
127+
128+
var currentPositionInList = currentPosition.ensureAssigned();
129+
if (currentPositionInList.entity() == targetPositionInList.entity()) { // The value is already in the list.
130+
131+
var valueCount = solutionView.countValues(variableMetaModel, currentPositionInList.entity());
132+
if (valueCount == 1) { // The value is the only value in the list; no change.
133+
return false;
134+
} else if (targetPositionInList.index() == valueCount) { // Trying to move the value past the end of the list.
135+
return false;
136+
} else { // Same list, same position; ignore.
137+
return currentPositionInList.index() != targetPositionInList.index();
138+
}
139+
}
140+
141+
// We can move freely between entities, assuming the target entity accepts the value.
142+
return solutionView.isValueInRange(variableMetaModel, targetPositionInList.entity(), value);
143+
}
144+
118145
}

core/src/main/java/ai/timefold/solver/core/impl/move/streams/maybeapi/stream/UniDataStream.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ default UniDataStream<Solution_, A> ifNotExistsOther(Class<A> otherClass, BiData
142142
* <li>Bijectivity.
143143
* No two input tuples should map to the same output tuple,
144144
* or to tuples that are {@link Object#equals(Object) equal}.
145-
* Not following this recommendation creates a constraint stream with duplicate tuples,
145+
* Not following this recommendation creates a data stream with duplicate tuples,
146146
* and may force you to use {@link #distinct()} later, which comes at a performance cost.</li>
147147
* <li>Immutable data carriers.
148148
* The objects returned by the mapping function should be identified by their contents and nothing else.
@@ -153,13 +153,13 @@ default UniDataStream<Solution_, A> ifNotExistsOther(Class<A> otherClass, BiData
153153
* </ul>
154154
*
155155
* <p>
156-
* Simple example: assuming a constraint stream of tuples of {@code Person}s
156+
* Simple example: assuming a data stream of tuples of {@code Person}s
157157
* {@code [Ann(age = 20), Beth(age = 25), Cathy(age = 30)]},
158158
* calling {@code map(Person::getAge)} on such stream will produce a stream of {@link Integer}s
159159
* {@code [20, 25, 30]},
160160
*
161161
* <p>
162-
* Example with a non-bijective mapping function: assuming a constraint stream of tuples of {@code Person}s
162+
* Example with a non-bijective mapping function: assuming a data stream of tuples of {@code Person}s
163163
* {@code [Ann(age = 20), Beth(age = 25), Cathy(age = 30), David(age = 30), Eric(age = 20)]},
164164
* calling {@code map(Person::getAge)} on such stream will produce a stream of {@link Integer}s
165165
* {@code [20, 25, 30, 30, 20]}.
@@ -189,7 +189,7 @@ <ResultA_, ResultB_> BiDataStream<Solution_, ResultA_, ResultB_> map(UniDataMapp
189189
* (No two tuples will {@link Object#equals(Object) equal}.)
190190
*
191191
* <p>
192-
* By default, tuples going through a constraint stream are distinct.
192+
* By default, tuples going through a data stream are distinct.
193193
* However, operations such as {@link #map(UniDataMapper)} may create a stream which breaks that promise.
194194
* By calling this method on such a stream,
195195
* duplicate copies of the same tuple will be omitted at a performance cost.

0 commit comments

Comments
 (0)