Skip to content

feat: Improve ScoreAnalysis debug information #923

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

Merged
merged 9 commits into from
Jul 5, 2024
Merged

feat: Improve ScoreAnalysis debug information #923

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2dab802
feat: add summary to score analysis
zepfred Jun 27, 2024
d44a691
docs: improve score analysis documentation
zepfred Jun 27, 2024
cd04760
chore: improve score analysis tests
zepfred Jun 27, 2024
1de9105
chore: add matchCount method
zepfred Jun 27, 2024
1e39882
feat: allow to analyze uninitialized solutions
zepfred Jun 27, 2024
74eee2b
chore: minor formatting changes
zepfred Jun 28, 2024
1a3fd22
fix: addressing PR comments
zepfred Jul 1, 2024
77d4c26
chore: minor fix
zepfred Jul 1, 2024
7c1f19f
chore: minor fix
zepfred Jul 1, 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
82 changes: 71 additions & 11 deletions core/src/main/java/ai/timefold/solver/core/api/score/analysis/ConstraintAnalysis.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package ai.timefold.solver.core.api.score.analysis;

import static ai.timefold.solver.core.api.score.analysis.ScoreAnalysis.DEFAULT_SUMMARY_CONSTRAINT_MATCH_LIMIT;
import static java.util.Comparator.comparing;

import java.util.Comparator;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import ai.timefold.solver.core.api.score.Score;
Expand Down Expand Up @@ -56,6 +59,21 @@ static <Score_ extends Score<Score_>> ConstraintAnalysis<Score_> of(ConstraintRe
Objects.requireNonNull(score);
}

/**
* Return the match count of the constraint.
*
* @throws IllegalStateException if the {@link ConstraintAnalysis#matches()} is null
*/
public int matchCount() {
if (matches == null) {
throw new IllegalArgumentException("""
The constraint matches must be non-null.
Maybe use ScoreAnalysisFetchPolicy.FETCH_ALL to request the score analysis
""");
}
return matches.size();
}

ConstraintAnalysis<Score_> negate() {
if (matches == null) {
return ConstraintAnalysis.of(constraintRef, weight.negate(), score.negate());
Expand All @@ -72,9 +90,9 @@ static <Score_ extends Score<Score_>> ConstraintAnalysis<Score_> diff(
ConstraintAnalysis<Score_> otherConstraintAnalysis) {
if (constraintAnalysis == null) {
if (otherConstraintAnalysis == null) {
throw new IllegalStateException("""
Impossible state: none of the score explanations provided constraint matches for a constraint (%s).
""".formatted(constraintRef));
throw new IllegalStateException(
"Impossible state: none of the score explanations provided constraint matches for a constraint (%s)."
.formatted(constraintRef));
}
// No need to compute diff; this constraint is not present in this score explanation.
return otherConstraintAnalysis.negate();
Expand All @@ -85,9 +103,9 @@ static <Score_ extends Score<Score_>> ConstraintAnalysis<Score_> diff(
var matchAnalyses = constraintAnalysis.matches();
var otherMatchAnalyses = otherConstraintAnalysis.matches();
if ((matchAnalyses == null && otherMatchAnalyses != null) || (matchAnalyses != null && otherMatchAnalyses == null)) {
throw new IllegalStateException("""
Impossible state: Only one of the score analyses (%s, %s) provided match analyses for a constraint (%s)."""
.formatted(constraintAnalysis, otherConstraintAnalysis, constraintRef));
throw new IllegalStateException(
"Impossible state: Only one of the score analyses (%s, %s) provided match analyses for a constraint (%s)."
.formatted(constraintAnalysis, otherConstraintAnalysis, constraintRef));
}
// Compute the diff.
var constraintWeightDifference = constraintAnalysis.weight().subtract(otherConstraintAnalysis.weight());
Expand All @@ -104,9 +122,9 @@ static <Score_ extends Score<Score_>> ConstraintAnalysis<Score_> diff(
var otherMatchAnalysis = otherMatchAnalysisMap.get(justification);
if (matchAnalysis == null) {
if (otherMatchAnalysis == null) {
throw new IllegalStateException("""
Impossible state: none of the match analyses provided for a constraint (%s).
""".formatted(constraintRef));
throw new IllegalStateException(
"Impossible state: none of the match analyses provided for a constraint (%s)."
.formatted(constraintRef));
}
// No need to compute diff; this match is not present in this score explanation.
return otherMatchAnalysis.negate();
Expand All @@ -118,7 +136,7 @@ static <Score_ extends Score<Score_>> ConstraintAnalysis<Score_> diff(
justification);
}
})
.collect(Collectors.toList());
.toList();
return new ConstraintAnalysis<>(constraintRef, constraintWeightDifference, scoreDifference, result);
}

Expand Down Expand Up @@ -156,6 +174,48 @@ public String constraintName() {
return constraintRef.constraintName();
}

/**
* Returns a diagnostic text that explains part of the score quality through the {@link ConstraintAnalysis} API.
* The string is built fresh every time the method is called.
*
* @return never null
*/
@SuppressWarnings("java:S3457")
public String summarize() {
var summary = new StringBuilder();
summary.append("""
Explanation of score (%s):
Constraint matches:
""".formatted(score));
Comparator<MatchAnalysis<Score_>> matchScoreComparator = comparing(MatchAnalysis::score);

var constraintMatches = matches();
if (constraintMatches == null) {
throw new IllegalArgumentException("""
The constraint matches must be non-null.
Maybe use ScoreAnalysisFetchPolicy.FETCH_ALL to request the score analysis
""");
}
if (constraintMatches.isEmpty()) {
summary.append(
"%8s%s: constraint (%s) has no matches.\n".formatted(" ", score().toShortString(),
constraintRef().constraintName()));
} else {
summary.append("%8s%s: constraint (%s) has %s matches:\n".formatted(" ", score().toShortString(),
constraintRef().constraintName(), constraintMatches.size()));
}
constraintMatches.stream()
.sorted(matchScoreComparator)
.limit(DEFAULT_SUMMARY_CONSTRAINT_MATCH_LIMIT)
.forEach(match -> summary.append("%12S%s: justified with (%s)\n".formatted(" ", match.score().toShortString(),
match.justification())));
if (constraintMatches.size() > DEFAULT_SUMMARY_CONSTRAINT_MATCH_LIMIT) {
summary.append("%12s%s\n".formatted(" ", "..."));
}

return summary.toString();
}

@Override
public String toString() {
if (matches == null) {
Expand Down
70 changes: 70 additions & 0 deletions core/src/main/java/ai/timefold/solver/core/api/score/analysis/ScoreAnalysis.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package ai.timefold.solver.core.api.score.analysis;

import static java.util.Comparator.comparing;

import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
Expand Down Expand Up @@ -53,6 +55,8 @@
public record ScoreAnalysis<Score_ extends Score<Score_>>(Score_ score,
Map<ConstraintRef, ConstraintAnalysis<Score_>> constraintMap) {

static final int DEFAULT_SUMMARY_CONSTRAINT_MATCH_LIMIT = 3;

public ScoreAnalysis {
Objects.requireNonNull(score, "score");
Objects.requireNonNull(constraintMap, "constraintMap");
Expand Down Expand Up @@ -141,4 +145,70 @@ public Collection<ConstraintAnalysis<Score_>> constraintAnalyses() {
return constraintMap.values();
}

/**
* Returns a diagnostic text that explains the solution through the {@link ConstraintAnalysis} API to identify which
* constraints cause that score quality.
* The string is built fresh every time the method is called.
* <p>
* In case of an {@link Score#isFeasible() infeasible} solution, this can help diagnose the cause of that.
*
* <p>
* Do not parse the return value, its format may change without warning.
* Instead, provide this information in a UI or a service,
* use {@link ScoreAnalysis#constraintAnalyses()}
* and convert those into a domain-specific API.
*
* @return never null
*/
@SuppressWarnings("java:S3457")
public String summarize() {
StringBuilder summary = new StringBuilder();
summary.append("""
Explanation of score (%s):
Constraint matches:
""".formatted(score));
Comparator<ConstraintAnalysis<Score_>> constraintsScoreComparator = comparing(ConstraintAnalysis::score);
Comparator<MatchAnalysis<Score_>> matchScoreComparator = comparing(MatchAnalysis::score);

constraintAnalyses().stream()
.sorted(constraintsScoreComparator)
.forEach(constraint -> {
var matches = constraint.matches();
if (matches == null) {
throw new IllegalArgumentException("""
The constraint matches must be non-null.
Maybe use ScoreAnalysisFetchPolicy.FETCH_ALL to request the score analysis
""");
}
if (matches.isEmpty()) {
summary.append(
"%8s%s: constraint (%s) has no matches.\n".formatted(" ", constraint.score().toShortString(),
constraint.constraintRef().constraintName()));
} else {
summary.append(
"%8s%s: constraint (%s) has %s matches:\n".formatted(" ", constraint.score().toShortString(),
constraint.constraintRef().constraintName(), matches.size()));
}
matches.stream()
.sorted(matchScoreComparator)
.limit(DEFAULT_SUMMARY_CONSTRAINT_MATCH_LIMIT)
.forEach(match -> summary
.append("%12s%s: justified with (%s)\n".formatted(" ", match.score().toShortString(),
match.justification())));
if (matches.size() > DEFAULT_SUMMARY_CONSTRAINT_MATCH_LIMIT) {
summary.append("%12s%s\n".formatted(" ", "..."));
}
});

return summary.toString();
}

public boolean isSolutionInitialized() {
return score().isSolutionInitialized();
}

@Override
public String toString() {
return "Score analysis of score %s with %d constraints.".formatted(score, constraintMap.size());
}
}
12 changes: 2 additions & 10 deletions core/src/main/java/ai/timefold/solver/core/impl/score/director/InnerScoreDirector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,16 +342,8 @@ default ScoreAnalysis<Score_> buildScoreAnalysis(boolean analyzeConstraintMatche
*/
default ScoreAnalysis<Score_> buildScoreAnalysis(boolean analyzeConstraintMatches, ScoreAnalysisMode mode) {
var score = calculateScore();
switch (Objects.requireNonNull(mode)) {
case RECOMMENDATION_API -> score = score.withInitScore(0);
case DEFAULT -> {
if (!score.isSolutionInitialized()) {
throw new IllegalArgumentException("""
Cannot analyze solution (%s) as it is not initialized (%s).
Maybe run the solver first?"""
.formatted(getWorkingSolution(), score));
}
}
if (Objects.requireNonNull(mode) == ScoreAnalysisMode.RECOMMENDATION_API) {
score = score.withInitScore(0);
}
var constraintAnalysisMap = new TreeMap<ConstraintRef, ConstraintAnalysis<Score_>>();
for (var constraintMatchTotal : getConstraintMatchTotalMap().values()) {
Expand Down
127 changes: 127 additions & 0 deletions core/src/test/java/ai/timefold/solver/core/api/score/analysis/ScoreAnalysisTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package ai.timefold.solver.core.api.score.analysis;

import static ai.timefold.solver.core.impl.score.director.InnerScoreDirector.getConstraintAnalysis;
import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
import static org.assertj.core.api.SoftAssertions.assertSoftly;

import java.util.Arrays;
Expand All @@ -26,6 +28,131 @@ void empty() {
softly.assertThat(diff.score()).isEqualTo(SimpleScore.of(0));
softly.assertThat(diff.constraintMap()).isEmpty();
});

var summary = scoreAnalysis.summarize();
assertThat(summary)
.isEqualTo("""
Explanation of score (0):
Constraint matches:
""");
}

@Test
void summarize() {
var constraintPackage = "constraintPackage";
var constraintName1 = "constraint1";
var constraintName2 = "constraint2";
var constraintName3 = "constraint3";
var constraintId1 = ConstraintRef.of(constraintPackage, constraintName1);
var constraintId2 = ConstraintRef.of(constraintPackage, constraintName2);
var constraintId3 = ConstraintRef.of(constraintPackage, constraintName3);

var constraintMatchTotal = new DefaultConstraintMatchTotal<>(constraintId1, SimpleScore.of(1));
addConstraintMatch(constraintMatchTotal, SimpleScore.of(2), "A", "B", "C");
addConstraintMatch(constraintMatchTotal, SimpleScore.of(4), "A", "B");
addConstraintMatch(constraintMatchTotal, SimpleScore.of(6), "B", "C");
addConstraintMatch(constraintMatchTotal, SimpleScore.of(7), "C");
addConstraintMatch(constraintMatchTotal, SimpleScore.of(8));
var constraintMatchTotal2 = new DefaultConstraintMatchTotal<>(constraintId2, SimpleScore.of(3));
addConstraintMatch(constraintMatchTotal2, SimpleScore.of(3), "B", "C", "D");
addConstraintMatch(constraintMatchTotal2, SimpleScore.of(6), "B", "C");
addConstraintMatch(constraintMatchTotal2, SimpleScore.of(9), "C", "D");
addConstraintMatch(constraintMatchTotal2, SimpleScore.of(10), "D");
addConstraintMatch(constraintMatchTotal2, SimpleScore.of(12));
var emptyConstraintMatchTotal1 = new DefaultConstraintMatchTotal<>(constraintId3, SimpleScore.of(0));
var constraintAnalysisMap = Map.of(
constraintMatchTotal.getConstraintRef(), getConstraintAnalysis(constraintMatchTotal, true),
constraintMatchTotal2.getConstraintRef(), getConstraintAnalysis(constraintMatchTotal2, true),
emptyConstraintMatchTotal1.getConstraintRef(), getConstraintAnalysis(emptyConstraintMatchTotal1, true));
var scoreAnalysis = new ScoreAnalysis<>(SimpleScore.of(67), constraintAnalysisMap);

// Single constraint analysis
var constraintSummary = constraintAnalysisMap.get(constraintMatchTotal.getConstraintRef()).summarize();
assertThat(constraintSummary)
.isEqualTo("""
Explanation of score (27):
Constraint matches:
27: constraint (constraint1) has 5 matches:
2: justified with ([A, B, C])
4: justified with ([A, B])
6: justified with ([B, C])
...
""");

// Complete score analysis
var summary = scoreAnalysis.summarize();
assertThat(scoreAnalysis.getConstraintAnalysis(constraintPackage, constraintName1).matchCount()).isEqualTo(5);
assertThat(summary)
.isEqualTo("""
Explanation of score (67):
Constraint matches:
0: constraint (constraint3) has no matches.
27: constraint (constraint1) has 5 matches:
2: justified with ([A, B, C])
4: justified with ([A, B])
6: justified with ([B, C])
...
40: constraint (constraint2) has 5 matches:
3: justified with ([B, C, D])
6: justified with ([B, C])
9: justified with ([C, D])
...
""");
}

@Test
void summarizeUninitializedSolution() {
var constraintPackage = "constraintPackage";
var constraintName1 = "constraint1";
var constraintName2 = "constraint2";
var constraintId1 = ConstraintRef.of(constraintPackage, constraintName1);
var constraintId2 = ConstraintRef.of(constraintPackage, constraintName2);

var constraintMatchTotal = new DefaultConstraintMatchTotal<>(constraintId1, SimpleScore.of(0));
var constraintMatchTotal2 = new DefaultConstraintMatchTotal<>(constraintId2, SimpleScore.of(0));
var constraintAnalysisMap = Map.of(
constraintMatchTotal.getConstraintRef(), getConstraintAnalysis(constraintMatchTotal, true),
constraintMatchTotal2.getConstraintRef(), getConstraintAnalysis(constraintMatchTotal2, true));
var scoreAnalysis = new ScoreAnalysis<>(SimpleScore.ofUninitialized(3, 0), constraintAnalysisMap);

// Single constraint analysis
var constraintSummary = constraintAnalysisMap.get(constraintMatchTotal.getConstraintRef()).summarize();
assertThat(constraintSummary)
.isEqualTo("""
Explanation of score (0):
Constraint matches:
0: constraint (constraint1) has no matches.
""");

// Complete score analysis
var summary = scoreAnalysis.summarize();
assertThat(scoreAnalysis.getConstraintAnalysis(constraintPackage, constraintName1).matchCount()).isZero();
assertThat(summary)
.isEqualTo("""
Explanation of score (3init/0):
Constraint matches:
0: constraint (constraint1) has no matches.
0: constraint (constraint2) has no matches.
""");
}

@Test
void failFastSummarize() {
var constraintPackage = "constraintPackage";
var constraintName1 = "constraint1";
var constraintId1 = ConstraintRef.of(constraintPackage, constraintName1);

var constraintMatchTotal = new DefaultConstraintMatchTotal<>(constraintId1, SimpleScore.of(1));
addConstraintMatch(constraintMatchTotal, SimpleScore.of(2), "A", "B", "C");
var constraintAnalysisMap = Map.of(
constraintMatchTotal.getConstraintRef(), getConstraintAnalysis(constraintMatchTotal, false));
var scoreAnalysis = new ScoreAnalysis<>(SimpleScore.of(3), constraintAnalysisMap);

assertThatThrownBy(scoreAnalysis::summarize)
.hasMessageContaining("The constraint matches must be non-null");

assertThatThrownBy(() -> constraintAnalysisMap.values().stream().findFirst().get().matchCount())
.hasMessageContaining("The constraint matches must be non-null");
}

@Test
Expand Down
Loading
Loading