Skip to content

Commit

Permalink
chore: deprecate constraint package
Browse files Browse the repository at this point in the history
  • Loading branch information
triceo committed Jul 10, 2024
1 parent 0245d84 commit f8a7c19
Show file tree
Hide file tree
Showing 31 changed files with 568 additions and 190 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.TimeUnit;
Expand All @@ -20,7 +19,6 @@
import ai.timefold.solver.core.impl.phase.scope.AbstractStepScope;
import ai.timefold.solver.core.impl.score.definition.ScoreDefinition;
import ai.timefold.solver.core.impl.solver.scope.SolverScope;
import ai.timefold.solver.core.impl.util.Pair;

import io.micrometer.core.instrument.Gauge;
import io.micrometer.core.instrument.Meter;
Expand All @@ -31,6 +29,9 @@
public class StatisticRegistry<Solution_> extends SimpleMeterRegistry
implements PhaseLifecycleListener<Solution_> {

private static final String CONSTRAINT_PACKAGE_TAG = "constraint.package";
private static final String CONSTRAINT_NAME_TAG = "constraint.name";

List<BiConsumer<Long, AbstractStepScope<Solution_>>> stepMeterListenerList = new ArrayList<>();
List<BiConsumer<Long, AbstractStepScope<Solution_>>> bestSolutionMeterListenerList = new ArrayList<>();
AbstractStepScope<Solution_> bestSolutionStepScope = null;
Expand Down Expand Up @@ -102,25 +103,21 @@ public void extractScoreFromMeters(SolverMetric metric, Tags runId, Consumer<Sco
@SuppressWarnings({ "rawtypes", "unchecked" })
public void extractConstraintSummariesFromMeters(SolverMetric metric, Tags runId,
Consumer<ConstraintSummary<?>> constraintMatchTotalConsumer) {
Set<Meter.Id> meterIds = getMeterIds(metric, runId);
Set<Pair<String, String>> constraintPackageNamePairs = new HashSet<>();
// Add the constraint ids from the meter ids
meterIds.forEach(meterId -> constraintPackageNamePairs
.add(new Pair<>(meterId.getTag("constraint.package"), meterId.getTag("constraint.name"))));
constraintPackageNamePairs.forEach(constraintPackageNamePair -> {
String constraintPackage = constraintPackageNamePair.key();
String constraintName = constraintPackageNamePair.value();
Tags constraintMatchTotalRunId = runId.and("constraint.package", constraintPackage)
.and("constraint.name", constraintName);
// Get the score from the corresponding constraint package and constraint name meters
extractScoreFromMeters(metric, constraintMatchTotalRunId,
// Get the count gauge (add constraint package and constraint name to the run tags)
score -> getGaugeValue(metric.getMeterId() + ".count",
constraintMatchTotalRunId,
count -> constraintMatchTotalConsumer.accept(
new ConstraintSummary(ConstraintRef.of(constraintPackage, constraintName), score,
count.intValue()))));
});
getMeterIds(metric, runId)
.stream()
.map(meterId -> ConstraintRef.of(meterId.getTag(CONSTRAINT_PACKAGE_TAG), meterId.getTag(CONSTRAINT_NAME_TAG)))
.distinct()
.forEach(constraintRef -> {
var constraintMatchTotalRunId = runId.and(CONSTRAINT_PACKAGE_TAG, constraintRef.packageName())
.and(CONSTRAINT_NAME_TAG, constraintRef.constraintName());
// Get the score from the corresponding constraint package and constraint name meters
extractScoreFromMeters(metric, constraintMatchTotalRunId,
// Get the count gauge (add constraint package and constraint name to the run tags)
score -> getGaugeValue(metric.getMeterId() + ".count", constraintMatchTotalRunId,
count -> constraintMatchTotalConsumer
.accept(new ConstraintSummary(constraintRef, score, count.intValue()))));
});
}

public void getGaugeValue(SolverMetric metric, Tags runId, Consumer<Number> gaugeConsumer) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,8 @@ public long getTimeMillisSpent() {
return timeMillisSpent;
}

public String getConstraintPackage() {
return constraintRef.packageName();
}

public String getConstraintName() {
return constraintRef.constraintName();
public ConstraintRef getConstraintRef() {
return constraintRef;
}

public int getConstraintMatchCount() {
Expand All @@ -39,13 +35,9 @@ public Score getScoreTotal() {
return scoreTotal;
}

public String getConstraintId() {
return constraintRef.constraintId();
}

@Override
public String toCsvLine() {
return buildCsvLineWithStrings(timeMillisSpent, getConstraintPackage(), getConstraintName(),
return buildCsvLineWithStrings(timeMillisSpent, constraintRef.packageName(), constraintRef.constraintName(),
Integer.toString(constraintMatchCount), scoreTotal.toString());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ protected List<LineChart<Long, Double>> generateCharts(BenchmarkReport benchmark
builderList.add(new LineChart.Builder<>());
}
LineChart.Builder<Long, Double> builder = builderList.get(i);
String seriesLabel = point.getConstraintName() + " weight";
String seriesLabel = point.getConstraintRef().constraintName() + " weight";
// Only add changes
double lastValue = (builder.count(seriesLabel) == 0) ? 0.0 : builder.getLastValue(seriesLabel);
if (levelValues[i] != lastValue) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,8 @@ public long getTimeMillisSpent() {
return timeMillisSpent;
}

public String getConstraintPackage() {
return constraintRef.packageName();
}

public String getConstraintName() {
return constraintRef.constraintName();
public ConstraintRef getConstraintRef() {
return constraintRef;
}

public int getConstraintMatchCount() {
Expand All @@ -39,13 +35,9 @@ public Score getScoreTotal() {
return scoreTotal;
}

public String getConstraintId() {
return constraintRef.constraintId();
}

@Override
public String toCsvLine() {
return buildCsvLineWithStrings(timeMillisSpent, getConstraintPackage(), getConstraintName(),
return buildCsvLineWithStrings(timeMillisSpent, constraintRef.packageName(), constraintRef.constraintName(),
Integer.toString(constraintMatchCount), scoreTotal.toString());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ protected List<LineChart<Long, Double>> generateCharts(BenchmarkReport benchmark
builderList.add(new LineChart.Builder<>());
}
LineChart.Builder<Long, Double> builder = builderList.get(i);
String seriesLabel = point.getConstraintName() + " weight";
String seriesLabel = point.getConstraintRef().constraintName() + " weight";
// Only add changes
double lastValue = (builder.count(seriesLabel) == 0) ? 0.0 : builder.getLastValue(seriesLabel);
if (levelValues[i] != lastValue) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ protected void runTest(SoftAssertions assertions, List<ConstraintMatchTotalBestS
assertions.assertThat(outputPoints)
.hasSize(1)
.first()
.matches(s -> Objects.equals(s.getConstraintId(), "CN/CP"), "Constraint IDs do not match.")
.matches(s -> Objects.equals(s.getConstraintRef().constraintId(), "CN/CP"), "Constraint IDs do not match.")
.matches(s -> s.getConstraintMatchCount() == Integer.MAX_VALUE, "Constraint match counts do not match.")
.matches(s -> s.getScoreTotal().equals(SimpleScore.of(Integer.MAX_VALUE)), "Scores do not match.")
.matches(s -> s.getTimeMillisSpent() == Long.MAX_VALUE, "Millis do not match.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ protected void runTest(SoftAssertions assertions, List<ConstraintMatchTotalStepS
assertions.assertThat(outputPoints)
.hasSize(1)
.first()
.matches(s -> Objects.equals(s.getConstraintId(), "CN/CP"), "Constraint IDs do not match.")
.matches(s -> Objects.equals(s.getConstraintRef().constraintId(), "CN/CP"), "Constraint IDs do not match.")
.matches(s -> s.getConstraintMatchCount() == Integer.MAX_VALUE, "Constraint match counts do not match.")
.matches(s -> s.getScoreTotal().equals(SimpleScore.of(Integer.MAX_VALUE)), "Scores do not match.")
.matches(s -> s.getTimeMillisSpent() == Long.MAX_VALUE, "Millis do not match.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
* This is the default for every {@link ConstraintWeight#constraintPackage()} in the annotated class.
*
* @return defaults to the annotated class's package.
* @deprecated Leave empty and let the solver provide the default. Do not rely on constraint package in user code.
*/
@Deprecated(forRemoval = true, since = "1.13.0")
String constraintPackage() default "";

}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@
* concatenated with "/" and {@link #value() the constraint name}.
*
* @return defaults to {@link ConstraintConfiguration#constraintPackage()}
* @deprecated Leave empty and let the solver provide the default. Do not rely on constraint package in user code.
*/
@Deprecated(forRemoval = true, since = "1.13.0")
String constraintPackage() default "";

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,9 @@ static <Score_ extends Score<Score_>> ConstraintAnalysis<Score_> diff(
* Return package name of the constraint that this analysis is for.
*
* @return equal to {@code constraintRef.packageName()}
* @deprecated Do not rely on constraint package in user code.
*/
@Deprecated(forRemoval = true, since = "1.13.0")
public String constraintPackage() {
return constraintRef.packageName();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,41 @@ public ConstraintAnalysis<Score_> getConstraintAnalysis(ConstraintRef constraint
* @param constraintPackage never null
* @param constraintName never null
* @return null if no constraint matches of such constraint are present
* @deprecated Use {@link #getConstraintAnalysis(String)} instead.
*/
@Deprecated(forRemoval = true, since = "1.13.0")
public ConstraintAnalysis<Score_> getConstraintAnalysis(String constraintPackage, String constraintName) {
return getConstraintAnalysis(ConstraintRef.of(constraintPackage, constraintName));
}

/**
* As defined by {@link #getConstraintAnalysis(ConstraintRef)}.
*
* @param constraintName never null
* @return null if no constraint matches of such constraint are present
* @throws IllegalStateException if multiple constraints with the same name are present,
* which is possible if they are in different constraint packages.
* Constraint packages are deprecated, we recommend avoiding them and instead naming constraints uniquely.
* If you must use constraint packages, see {@link #getConstraintAnalysis(String, String)}
* (also deprecated) and reach out to us to discuss your use case.
*/
public ConstraintAnalysis<Score_> getConstraintAnalysis(String constraintName) {
var constraintAnalysisList = constraintMap.entrySet()
.stream()
.filter(entry -> entry.getKey().constraintName().equals(constraintName))
.map(Map.Entry::getValue)
.toList();
return switch (constraintAnalysisList.size()) {
case 0 -> null;
case 1 -> constraintAnalysisList.get(0);
default -> throw new IllegalStateException("""
Multiple constraints with the same name (%s) are present in the score analysis.
This may be caused by the use of multiple constraint packages, a deprecated feature.
Please avoid using constraint packages and keep constraint names unique."""
.formatted(constraintName));
};
}

/**
* Compare this {@link ScoreAnalysis} to another {@link ScoreAnalysis}
* and retrieve the difference between them.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
* @param packageName The constraint package is the namespace of the constraint.
* When using a {@link ConstraintConfiguration},
* it is equal to the {@link ConstraintWeight#constraintPackage()}.
* It is not recommended for the user to set this, or to read its value;
* instead, the user should use whatever the solver provided as default and not rely on this information at all.
* The entire concept of constraint package is likely to be removed in a future version of the solver.
* @param constraintName The constraint name.
* It might not be unique, but {@link #constraintId()} is unique.
* When using a {@link ConstraintConfiguration},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ public interface ConstraintBuilder {
* @param constraintName never null, shows up in {@link ConstraintMatchTotal} during score justification
* @param constraintPackage never null
* @return never null
* @deprecated Constraint package should no longer be used, use {@link #asConstraint(String)} instead.
*/
@Deprecated(forRemoval = true, since = "1.13.0")
Constraint asConstraint(String constraintPackage, String constraintName);

}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ public interface ConstraintFactory {
* otherwise the package of the {@link PlanningSolution} class.
*
* @return never null
* @deprecated Do not rely on any constraint package in user code.
*/
@Deprecated(forRemoval = true, since = "1.13.0")
String getDefaultConstraintPackage();

// ************************************************************************
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,18 @@
public final class BavetConstraintFactory<Solution_>
extends InnerConstraintFactory<Solution_, BavetConstraint<Solution_>> {

/**
* Used for code in no package, also called the "unnamed package".
* Classes here can only be instantiated via reflection,
* they cannot be imported and used directly.
* But still, in corner cases such as Kotlin notebooks,
* all code is in the unnamed package.
* Assume a constraint provider under these conditions,
* where asConstraint(...) only specifies constraint name, not constraint package.
* In this situation, the default constraint package is used.
*/
private static final String DEFAULT_CONSTRAINT_PACKAGE = "unnamed.package";

private final SolutionDescriptor<Solution_> solutionDescriptor;
private final EnvironmentMode environmentMode;
private final String defaultConstraintPackage;
Expand All @@ -33,13 +45,21 @@ public BavetConstraintFactory(SolutionDescriptor<Solution_> solutionDescriptor,
ConstraintConfigurationDescriptor<Solution_> configurationDescriptor = solutionDescriptor
.getConstraintConfigurationDescriptor();
if (configurationDescriptor == null) {
Package pack = solutionDescriptor.getSolutionClass().getPackage();
defaultConstraintPackage = (pack == null) ? "" : pack.getName();
defaultConstraintPackage = determineDefaultConstraintPackage(solutionDescriptor.getSolutionClass().getPackage());
} else {
defaultConstraintPackage = configurationDescriptor.getConstraintPackage();
defaultConstraintPackage = determineDefaultConstraintPackage(configurationDescriptor.getConstraintPackage());
}
}

private static String determineDefaultConstraintPackage(Package pkg) {
var asString = pkg == null ? "" : pkg.getName();
return determineDefaultConstraintPackage(asString);
}

private static String determineDefaultConstraintPackage(String constraintPackage) {
return constraintPackage == null || constraintPackage.isEmpty() ? DEFAULT_CONSTRAINT_PACKAGE : constraintPackage;
}

public <Stream_ extends BavetAbstractConstraintStream<Solution_>> Stream_ share(Stream_ stream) {
return share(stream, t -> {
});
Expand Down
88 changes: 88 additions & 0 deletions core/src/test/java/TestdataInUnnamedPackageSolution.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import java.util.ArrayList;
import java.util.List;

import ai.timefold.solver.core.api.domain.solution.PlanningEntityCollectionProperty;
import ai.timefold.solver.core.api.domain.solution.PlanningScore;
import ai.timefold.solver.core.api.domain.solution.PlanningSolution;
import ai.timefold.solver.core.api.domain.solution.ProblemFactCollectionProperty;
import ai.timefold.solver.core.api.domain.valuerange.ValueRangeProvider;
import ai.timefold.solver.core.api.score.buildin.simple.SimpleScore;
import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor;
import ai.timefold.solver.core.impl.testdata.domain.TestdataEntity;
import ai.timefold.solver.core.impl.testdata.domain.TestdataObject;
import ai.timefold.solver.core.impl.testdata.domain.TestdataValue;

@PlanningSolution
public class TestdataInUnnamedPackageSolution extends TestdataObject {

public static SolutionDescriptor<TestdataInUnnamedPackageSolution> buildSolutionDescriptor() {
return SolutionDescriptor.buildSolutionDescriptor(TestdataInUnnamedPackageSolution.class, TestdataEntity.class);
}

public static TestdataInUnnamedPackageSolution generateSolution() {
return generateSolution(5, 7);
}

public static TestdataInUnnamedPackageSolution generateSolution(int valueListSize, int entityListSize) {
TestdataInUnnamedPackageSolution solution = new TestdataInUnnamedPackageSolution("Generated Solution 0");
List<TestdataValue> valueList = new ArrayList<>(valueListSize);
for (int i = 0; i < valueListSize; i++) {
TestdataValue value = new TestdataValue("Generated Value " + i);
valueList.add(value);
}
solution.setValueList(valueList);
List<TestdataEntity> entityList = new ArrayList<>(entityListSize);
for (int i = 0; i < entityListSize; i++) {
TestdataValue value = valueList.get(i % valueListSize);
TestdataEntity entity = new TestdataEntity("Generated Entity " + i, value);
entityList.add(entity);
}
solution.setEntityList(entityList);
return solution;
}

private List<TestdataValue> valueList;
private List<TestdataEntity> entityList;

private SimpleScore score;

public TestdataInUnnamedPackageSolution() {
}

public TestdataInUnnamedPackageSolution(String code) {
super(code);
}

@ValueRangeProvider(id = "valueRange")
@ProblemFactCollectionProperty
public List<TestdataValue> getValueList() {
return valueList;
}

public void setValueList(List<TestdataValue> valueList) {
this.valueList = valueList;
}

@PlanningEntityCollectionProperty
public List<TestdataEntity> getEntityList() {
return entityList;
}

public void setEntityList(List<TestdataEntity> entityList) {
this.entityList = entityList;
}

@PlanningScore
public SimpleScore getScore() {
return score;
}

public void setScore(SimpleScore score) {
this.score = score;
}

// ************************************************************************
// Complex methods
// ************************************************************************

}
Loading

0 comments on commit f8a7c19

Please sign in to comment.