Repository navigation
BiConstraintCollector doesn't #509
Description
Describe the bug
I'm implementing a custom BiConstraintCollector for use in groupBy() constraint streams.
For some reason, the elements are not collected, the provided ResultContainer remains empty, and the constraint doesn't penalize.
It might very well be that I'm using the API wrong (e.g. the two consecutive calls of groupBy come to mind). I'll gladly take advice in any direction.
Expected behavior
Invocation of the Runnables for each tuple. Passing of the test.
Actual behavior
The Runnables are not invoked. The test fails with
java.lang.AssertionError: Broken expectation.
Constraint: ai.timefold.solver.examples.cloudbalancing.domain/differenceInRequiredCpu
Expected penalty: 1 (class java.lang.Integer)
Actual penalty: 0 (class java.lang.Integer)
Explanation of score (0hard/0soft):
Constraint matches:
0: constraint (differenceInRequiredCpu) has 1 matches:
0: justified with ([{}])
Indictments:
0: indicted with ({}) has 1 matches:
0: constraint (differenceInRequiredCpu)
To Reproduce
My minimal example is based on the CloudBalancing example, and introduces the following additional constraint (which seems odd in this particular example, but for demonstration purposes I hope it'll do):
For each computer, I want to penalize the scheduling of processes that have different cpu requirements. And I want to do this in a per-computer way, so the cost doesn't explode when scheduling lots of small tasks.
So I implemented a MinMaxCollector, with the intention to determine the minimum and maximum cpu requirements of scheduled processes per computer.
I forked the timefold-solver repo and added my BiConstraintCollector, the additional constraint and a unit test here:
https://github.com/thimmwork/timefold-solver/blob/4862eef96575fc061eb5b5cb0727b791790e184a/examples/src/test/java/ai/timefold/solver/examples/cloudbalancing/score/CloudBalancingConstraintProviderTest.java#L84C21-L84C21
This is the constraint I added to CloudBalancingConstraintProvider.
I'm using the computer.id as key and want to collect the min and max requiredCpuPower of all the processes assigned to each computer.
Constraint differenceInRequiredCpu(ConstraintFactory constraintFactory) {
return constraintFactory.forEach(CloudProcess.class)
.filter(cloudProcess -> cloudProcess.getComputer() != null)
.groupBy(cloudProcess -> cloudProcess.getComputer().getId(), cloudProcess -> cloudProcess.getRequiredCpuPower())
.groupBy(MinMaxCollector.collector())
.penalize(HardSoftScore.ONE_HARD,
cpuPowerByComputer -> penalizeByCpuDifference(cpuPowerByComputer.values()))
.asConstraint("differenceInRequiredCpu");
}
private Integer penalizeByCpuDifference(Collection<Pair<Integer, Integer>> minAndMaxCpuByComputer) {
return minAndMaxCpuByComputer.stream()
.mapToInt(minMax -> minMax.getRight() - minMax.getLeft())
// use the square to penalize bigger differences way more than smaller differences
.map(difference -> difference * difference)
.sum();
}
This is my Collector (for easier code analysis I'm using anonymous classes here)
public class MinMaxCollector<KEY, COMP extends Comparable<COMP>> {
private final Map<KEY, Pair<COMP, COMP>> minMaxPerKey = new HashMap<>();
public void collect(KEY key, COMP other) {
var minMax = minMaxPerKey.get(key);
var replace = false;
if (minMax == null) {
minMax = ImmutablePair.of(other, other);
replace = true;
} else if (minMax.getLeft().compareTo(other) > 0) {
minMax = ImmutablePair.of(other, minMax.getRight());
replace = true;
}
if (minMax.getRight().compareTo(other) < 0) {
minMax = ImmutablePair.of(minMax.getLeft(), other);
replace = true;
}
if (replace) {
minMaxPerKey.put(key, minMax);
}
}
public Map<KEY, Pair<COMP, COMP>> getMinMaxPerKey() {
return minMaxPerKey;
}
public static <KEY, COMP extends Comparable<COMP>>
BiConstraintCollector<KEY, COMP, MinMaxCollector<KEY, COMP>, Map<KEY, Pair<COMP, COMP>>> collector() {
return new BiConstraintCollector<>() {
@Override
public Supplier<MinMaxCollector<KEY, COMP>> supplier() {
return MinMaxCollector::new;
}
@Override
public TriFunction<MinMaxCollector<KEY, COMP>, KEY, COMP, Runnable> accumulator() {
return new TriFunction<MinMaxCollector<KEY, COMP>, KEY, COMP, Runnable>() {
@Override
public Runnable apply(MinMaxCollector<KEY, COMP> minMaxCollector, KEY key, COMP comp) {
return new Runnable() {
@Override
public void run() {
minMaxCollector.collect(key, comp);
}
};
}
};
}
@Override
public Function<MinMaxCollector<KEY, COMP>, Map<KEY, Pair<COMP, COMP>>> finisher() {
return MinMaxCollector::getMinMaxPerKey;
}
};
}
}
and a unit test that I added to CloudBalancingConstraintProviderTest:
@ConstraintProviderTest
void useThisTestToReproduce(ConstraintVerifier<CloudBalancingConstraintProvider, CloudBalance> constraintVerifier) {
CloudComputer computer1 = new CloudComputer(1, 1, 1, 1, 2);
CloudProcess process1 = new CloudProcess(1, 1, 1, 1);
CloudProcess process2 = new CloudProcess(2, 3, 1, 1);
process1.setComputer(computer1);
process2.setComputer(computer1);
constraintVerifier.verifyThat(CloudBalancingConstraintProvider::differenceInRequiredCpu)
.given(computer1, process1, process2)
.penalizesBy(1);
}
My expectation would be that the above unit test would trigger the same invocations as this test, which tests the collector directly:
@Test
public void biconstraintcollector_smoke_test() {
//given
Integer computerId = 1;
BiConstraintCollector<Integer, Integer, MinMaxCollector<Integer, Integer>, Map<Integer, Pair<Integer, Integer>>> biConstraintCollector = MinMaxCollector.collector();
//when
MinMaxCollector<Integer, Integer> minMaxCollector = biConstraintCollector.supplier().get();
TriFunction<MinMaxCollector<Integer, Integer>, Integer, Integer, Runnable> accumulator = biConstraintCollector.accumulator();
// A: min=1, max=10
accumulator.apply(minMaxCollector, computerId, 1).run();
accumulator.apply(minMaxCollector, computerId, 3).run();
//then
var finisher = biConstraintCollector.finisher().apply(minMaxCollector);
assertEquals(1, finisher.get(computerId).getLeft());
assertEquals(3, finisher.get(computerId).getRight());
}
This test works, so the problem is either outside of the BiConstraintCollector or in my understanding.
Environment
Timefold Solver Version or Git ref:
tested on solver 1.20, 1.4.0, 1.5.0 and git commit 490490a (current main)
Output of java -version:
openjdk 17.0.9 2023-10-17
OpenJDK Runtime Environment Temurin-17.0.9+9 (build 17.0.9+9)
OpenJDK 64-Bit Server VM Temurin-17.0.9+9 (build 17.0.9+9, mixed mode)
Output of uname -a or ver:
Darwin WYJTH5W7YK 22.6.0 Darwin Kernel Version 22.6.0: Fri Sep 15 13:41:28 PDT 2023; root:xnu-8796.141.3.700.8~1/RELEASE_ARM64_T6000 arm64
Additional information
Provide any and all other information which might be relevant to the issue.
Activity
triceo commentedon Dec 20, 2023
Hey there!
The problem appears to be in your accumulator. You need something like this:
In your accumulator, you're only returning the undo
Runnable, and that means that nothing gets accumulated. There is no code to actually accumulate anything.I'll convert this issue to a discussion, as my current understanding indicates this is not a solver problem. If there's still something to explain, we can continue there.