Skip to content

Commit

Permalink
Review
Browse files Browse the repository at this point in the history
  • Loading branch information
triceo committed Nov 12, 2024
1 parent 4e08586 commit a345ba5
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ protected MoveSelector<Solution_> buildBaseMoveSelector(HeuristicConfigPolicy<So
.buildValueSelector(configPolicy, entityDescriptor, minimumCacheType, selectionOrder);

if (!(sourceValueSelector instanceof EntityIndependentValueSelector<Solution_> castSourceValueSelector)) {
throw new IllegalArgumentException("The listChangeMoveSelector (" + config
+ ") for a list variable needs to be based on an "
+ EntityIndependentValueSelector.class.getSimpleName() + " (" + sourceValueSelector + ")."
+ " Check your valueSelectorConfig.");
throw new IllegalArgumentException("""
The listChangeMoveSelector (%s) for a list variable needs to be based on an %s (%s).
Check your valueSelectorConfig."""
.formatted(config, EntityIndependentValueSelector.class.getSimpleName(), sourceValueSelector));
}

var destinationSelector = DestinationSelectorFactory
Expand All @@ -74,9 +74,10 @@ protected MoveSelectorConfig<?> buildUnfoldedMoveSelectorConfig(HeuristicConfigP
onlyEntityDescriptor == null ? configPolicy.getSolutionDescriptor().getGenuineEntityDescriptors()
: Collections.singletonList(onlyEntityDescriptor);
if (entityDescriptors.size() > 1) {
throw new IllegalArgumentException("The listChangeMoveSelector (" + config
+ ") cannot unfold when there are multiple entities (" + entityDescriptors + ")."
+ " Please use one listChangeMoveSelector per each planning list variable.");
throw new IllegalArgumentException("""
The listChangeMoveSelector (%s) cannot unfold when there are multiple entities (%s).
Please use one listChangeMoveSelector per each planning list variable."""
.formatted(config, entityDescriptors));
}
var entityDescriptor = entityDescriptors.iterator().next();

Expand All @@ -93,24 +94,22 @@ protected MoveSelectorConfig<?> buildUnfoldedMoveSelectorConfig(HeuristicConfigP
.extractVariableDescriptor(configPolicy, entityDescriptor);
if (onlyVariableDescriptor != null && onlyDestinationVariableDescriptor != null) {
if (!onlyVariableDescriptor.isListVariable()) {
throw new IllegalArgumentException("The listChangeMoveSelector (" + config
+ ") is configured to use a planning variable (" + onlyVariableDescriptor
+ "), which is not a planning list variable."
+ " Either fix your annotations and use a @" + PlanningListVariable.class.getSimpleName()
+ " on the variable to make it work with listChangeMoveSelector"
+ " or use a changeMoveSelector instead.");
throw new IllegalArgumentException("""
The listChangeMoveSelector (%s) is configured to use a planning variable (%s), \
which is not a planning list variable.
Either fix your annotations and use a @%s on the variable to make it work with listChangeMoveSelector
or use a changeMoveSelector instead."""
.formatted(config, onlyVariableDescriptor, PlanningListVariable.class.getSimpleName()));
}
if (!onlyDestinationVariableDescriptor.isListVariable()) {
throw new IllegalArgumentException("The destinationSelector (" + destinationSelectorConfig
+ ") is configured to use a planning variable (" + onlyDestinationVariableDescriptor
+ "), which is not a planning list variable.");
throw new IllegalArgumentException(
"The destinationSelector (%s) is configured to use a planning variable (%s), which is not a planning list variable."
.formatted(destinationSelectorConfig, onlyDestinationVariableDescriptor));
}
if (onlyVariableDescriptor != onlyDestinationVariableDescriptor) {
throw new IllegalArgumentException("The listChangeMoveSelector's valueSelector ("
+ valueSelectorConfig
+ ") and destinationSelector's valueSelector ("
+ destinationValueSelectorConfig
+ ") must be configured for the same planning variable.");
throw new IllegalArgumentException(
"The listChangeMoveSelector's valueSelector (%s) and destinationSelector's valueSelector (%s) must be configured for the same planning variable."
.formatted(valueSelectorConfig, destinationValueSelectorConfig));
}
if (onlyEntityDescriptor != null) {
// No need for unfolding or deducing
Expand All @@ -125,12 +124,14 @@ protected MoveSelectorConfig<?> buildUnfoldedMoveSelectorConfig(HeuristicConfigP
.toList());
}
if (variableDescriptorList.isEmpty()) {
throw new IllegalArgumentException("The listChangeMoveSelector (" + config
+ ") cannot unfold because there are no planning list variables.");
throw new IllegalArgumentException(
"The listChangeMoveSelector (%s) cannot unfold because there are no planning list variables."
.formatted(config));
}
if (variableDescriptorList.size() > 1) {
throw new IllegalArgumentException("The listChangeMoveSelector (" + config
+ ") cannot unfold because there are multiple planning list variables.");
throw new IllegalArgumentException(
"The listChangeMoveSelector (%s) cannot unfold because there are multiple planning list variables."
.formatted(config));
}
var listChangeMoveSelectorConfig =
buildChildMoveSelectorConfig(variableDescriptorList.get(0), valueSelectorConfig, destinationSelectorConfig);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,7 @@ Check your solver configuration. If that class (%s) is not in the entityClassSet
protected boolean determineBaseRandomSelection(GenuineVariableDescriptor<Solution_> variableDescriptor,
SelectionCacheType resolvedCacheType, SelectionOrder resolvedSelectionOrder) {
return switch (resolvedSelectionOrder) {
case ORIGINAL -> false;
case SORTED, SHUFFLED, PROBABILISTIC ->
case ORIGINAL, SORTED, SHUFFLED, PROBABILISTIC ->
// baseValueSelector and lower should be ORIGINAL if they are going to get cached completely
false;
case RANDOM ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,11 @@ public Score_ calculateScore() {
variableListenerSupport.assertNotificationQueuesAreEmpty();
Score_ score = easyScoreCalculator.calculateScore(workingSolution);
if (!score.isSolutionInitialized()) {
throw new IllegalStateException("The score (" + this + ")'s initScore (" + score.initScore()
+ ") should be 0.\n"
+ "Maybe the score calculator (" + easyScoreCalculator.getClass() + ") is calculating "
+ "the initScore too, although it's the score director's responsibility.");
throw new IllegalStateException("""
The score (%s)'s initScore (%d) should be 0.
Maybe the score calculator (%s) is calculating the initScore too, \
although it's the score director's responsibility."""
.formatted(this, score.initScore(), easyScoreCalculator.getClass()));
}
int workingInitScore = getWorkingInitScore();
if (workingInitScore != 0) {
Expand All @@ -69,8 +70,8 @@ public Score_ calculateScore() {
*/
@Override
public Map<String, ConstraintMatchTotal<Score_>> getConstraintMatchTotalMap() {
throw new IllegalStateException(ConstraintMatch.class.getSimpleName()
+ " is not supported by " + EasyScoreDirector.class.getSimpleName() + ".");
throw new IllegalStateException("%s is not supported by %s."
.formatted(ConstraintMatch.class.getSimpleName(), EasyScoreDirector.class.getSimpleName()));
}

/**
Expand All @@ -81,8 +82,8 @@ public Map<String, ConstraintMatchTotal<Score_>> getConstraintMatchTotalMap() {
*/
@Override
public Map<Object, Indictment<Score_>> getIndictmentMap() {
throw new IllegalStateException(ConstraintMatch.class.getSimpleName()
+ " is not supported by " + EasyScoreDirector.class.getSimpleName() + ".");
throw new IllegalStateException("%s is not supported by %s."
.formatted(ConstraintMatch.class.getSimpleName(), EasyScoreDirector.class.getSimpleName()));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import static ai.timefold.solver.core.api.score.stream.Joiners.lessThan;
import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.toList;

import java.util.Arrays;
import java.util.List;
Expand Down Expand Up @@ -42,9 +41,9 @@ private <A> DefaultBiJoiner<A, A> buildLessThanId(Class<A> sourceClass) {
SolutionDescriptor<Solution_> solutionDescriptor = getSolutionDescriptor();
MemberAccessor planningIdMemberAccessor = solutionDescriptor.getPlanningIdAccessor(sourceClass);
if (planningIdMemberAccessor == null) {
throw new IllegalArgumentException("The fromClass (" + sourceClass + ") has no member with a @"
+ PlanningId.class.getSimpleName() + " annotation,"
+ " so the pairs cannot be made unique ([A,B] vs [B,A]).");
throw new IllegalArgumentException(
"The fromClass (%s) has no member with a @%s annotation, so the pairs cannot be made unique ([A,B] vs [B,A])."
.formatted(sourceClass, PlanningId.class.getSimpleName()));
}
Function<A, Comparable> planningIdGetter = planningIdMemberAccessor.getGetterFunction();
return (DefaultBiJoiner<A, A>) lessThan(planningIdGetter);
Expand Down Expand Up @@ -74,34 +73,38 @@ public <A> void assertValidFromType(Class<A> fromType) {
List<String> canonicalClassNameList = problemFactOrEntityClassSet.stream()
.map(Class::getCanonicalName)
.sorted()
.collect(toList());
throw new IllegalArgumentException("Cannot use class (" + fromType.getCanonicalName()
+ ") in a constraint stream as it is neither the same as, nor a superclass or superinterface of "
+ "one of planning entities or problem facts.\n"
+ "Ensure that all from(), join(), ifExists() and ifNotExists() building blocks only reference "
+ "classes assignable from planning entities or problem facts (" + canonicalClassNameList + ") "
+ "annotated on the planning solution (" + solutionDescriptor.getSolutionClass().getCanonicalName()
+ ").");
.toList();
throw new IllegalArgumentException("""
Cannot use class (%s) in a constraint stream as it is neither the same as, \
nor a superclass or superinterface of one of planning entities or problem facts.
Ensure that all from(), join(), ifExists() and ifNotExists() building blocks \
only reference classes assignable from planning entities \
or problem facts (%s) annotated on the planning solution (%s)."""
.formatted(fromType.getCanonicalName(), canonicalClassNameList,
solutionDescriptor.getSolutionClass().getCanonicalName()));
}
}

@SuppressWarnings("unchecked")
public List<Constraint_> buildConstraints(ConstraintProvider constraintProvider) {
Constraint[] constraints = Objects.requireNonNull(constraintProvider.defineConstraints(this),
() -> """
The constraintProvider class (%s)'s defineConstraints() must not return null."
Maybe return an empty array instead if there are no constraints."""
.formatted(constraintProvider.getClass()));
if (Arrays.stream(constraints).anyMatch(Objects::isNull)) {
throw new IllegalStateException("The constraintProvider class (" + constraintProvider.getClass()
+ ")'s defineConstraints() must not contain an element that is null.\n"
+ "Maybe don't include any null elements in the " + Constraint.class.getSimpleName() + " array.");
throw new IllegalStateException("""
The constraintProvider class (%s)'s defineConstraints() must not contain an element that is null.
Maybe don't include any null elements in the %s array."""
.formatted(constraintProvider.getClass(), Constraint.class.getSimpleName()));
}
// Fail fast on duplicate constraint IDs.
Map<ConstraintRef, List<Constraint>> constraintsPerIdMap =
Arrays.stream(constraints).collect(groupingBy(Constraint::getConstraintRef));
constraintsPerIdMap.forEach((constraintRef, duplicateConstraintList) -> {
if (duplicateConstraintList.size() > 1) {
throw new IllegalStateException("There are multiple constraints with the same ID (" + constraintRef + ").");
throw new IllegalStateException("There are multiple constraints with the same ID (%s)."
.formatted(constraintRef));
}
});
return Arrays.stream(constraints)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ public void serialize(ConstraintWeightOverrides<Score_> constraintWeightOverride
SerializerProvider serializerProvider) throws IOException {
generator.writeStartObject();
for (var constraintName : constraintWeightOverrides.getKnownConstraintNames()) {
var weight = constraintWeightOverrides.getConstraintWeight(constraintName);
generator.writeStringField(constraintName, Objects.requireNonNull(weight).toString());
var weight = Objects.requireNonNull(constraintWeightOverrides.getConstraintWeight(constraintName));
generator.writeStringField(constraintName, weight.toString());
}
generator.writeEndObject();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ private void loadSolverConfig(IncludeAbstractClassesEntityScanner entityScanner,
if (solverEntityClassList == null) {
solverConfig.setEntityClassList(entityScanner.findEntityClassList());
} else {
long entityClassCount = solverEntityClassList.stream()
var entityClassCount = solverEntityClassList.stream()
.filter(Objects::nonNull)
.count();
if (entityClassCount == 0L) {
Expand Down

0 comments on commit a345ba5

Please sign in to comment.