Skip to content

Commit

Permalink
refactor: improve propagation rule diagnostics
Browse files Browse the repository at this point in the history
  • Loading branch information
kris7t committed Jun 28, 2024
1 parent 6095f6e commit b52a4b0
Show file tree
Hide file tree
Showing 11 changed files with 104 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ public ModelSemantics tryCreateSemantics(Problem problem) {
var storeBuilder = ModelStore.builder()
.cancellationToken(cancellationToken)
.with(QueryInterpreterAdapter.builder())
.with(PropagationAdapter.builder())
.with(PropagationAdapter.builder()
.throwOnFatalRejection(false))
.with(ReasoningAdapter.builder()
.requiredInterpretations(Set.of(Concreteness.PARTIAL)));
initializer.configureStoreBuilder(storeBuilder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,7 @@ private void collectRule(RuleDefinition ruleDefinition, ModelStoreBuilder storeB
.orElseGet(() -> "::rule" + ruleCount);
ruleCount++;
var rule = toRule(name, ruleDefinition);
problemTrace.putRuleDefinition(ruleDefinition, rule);
switch (ruleDefinition.getKind()) {
case DECISION -> storeBuilder.tryGetAdapter(DesignSpaceExplorationBuilder.class)
.ifPresent(dseBuilder -> dseBuilder.transformation(rule));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import tools.refinery.language.model.problem.Node;
import tools.refinery.language.model.problem.Problem;
import tools.refinery.language.model.problem.Relation;
import tools.refinery.language.model.problem.RuleDefinition;
import tools.refinery.store.dse.transition.Rule;
import tools.refinery.store.reasoning.representation.AnyPartialSymbol;
import tools.refinery.store.reasoning.representation.PartialRelation;
import tools.refinery.store.reasoning.translator.TranslationException;
Expand All @@ -36,6 +38,8 @@ public interface ProblemTrace {

Relation getRelation(AnyPartialSymbol partialSymbol);

RuleDefinition getRuleDefinition(Rule rule);

RuntimeException wrapException(TranslationException translationException);

PartialRelation getPartialRelation(Relation relation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@
import org.eclipse.xtext.naming.QualifiedName;
import org.eclipse.xtext.scoping.IScope;
import org.eclipse.xtext.scoping.IScopeProvider;
import tools.refinery.language.model.problem.Node;
import tools.refinery.language.model.problem.Problem;
import tools.refinery.language.model.problem.ProblemPackage;
import tools.refinery.language.model.problem.Relation;
import tools.refinery.language.model.problem.*;
import tools.refinery.store.dse.transition.Rule;
import tools.refinery.store.reasoning.representation.AnyPartialSymbol;
import tools.refinery.store.reasoning.representation.PartialRelation;
import tools.refinery.store.reasoning.translator.TranslationException;
Expand Down Expand Up @@ -46,6 +44,7 @@ class ProblemTraceImpl implements ProblemTrace {
Collections.unmodifiableMap(mutableRelationTrace);
private final Map<AnyPartialSymbol, Relation> mutableInverseTrace = new HashMap<>();
private final Map<AnyPartialSymbol, Relation> inverseTrace = Collections.unmodifiableMap(mutableInverseTrace);
private final Map<Rule, RuleDefinition> mutableInverseRuleDefinitionTrace = new LinkedHashMap<>();

@Override
public Problem getProblem() {
Expand Down Expand Up @@ -129,6 +128,23 @@ public Relation getRelation(AnyPartialSymbol partialSymbol) {
return relation;
}

void putRuleDefinition(RuleDefinition ruleDefinition, Rule rule) {
var oldRuleDefinition = mutableInverseRuleDefinitionTrace.put(rule, ruleDefinition);
if (oldRuleDefinition != null) {
throw new TracedException(oldRuleDefinition, "Rule definition %s was already mapped to rule"
.formatted(rule.getName()));
}
}

@Override
public RuleDefinition getRuleDefinition(Rule rule) {
var ruleDefinition = mutableInverseRuleDefinitionTrace.get(rule);
if (ruleDefinition == null) {
throw new IllegalArgumentException("No definition for rule: " + rule.getName());
}
return ruleDefinition;
}

@Override
public RuntimeException wrapException(TranslationException translationException) {
var partialSymbol = translationException.getPartialSymbol();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.google.inject.Inject;
import org.eclipse.emf.common.util.Diagnostic;
import org.eclipse.emf.ecore.EObject;
import org.eclipse.emf.ecore.EStructuralFeature;
import org.eclipse.xtext.service.OperationCanceledManager;
import org.eclipse.xtext.util.CancelIndicator;
import org.eclipse.xtext.validation.CheckType;
Expand All @@ -18,11 +19,14 @@
import tools.refinery.generator.ModelSemantics;
import tools.refinery.generator.ModelSemanticsFactory;
import tools.refinery.language.model.problem.Problem;
import tools.refinery.language.model.problem.ProblemPackage;
import tools.refinery.language.model.problem.ScopeDeclaration;
import tools.refinery.language.semantics.ProblemTrace;
import tools.refinery.language.semantics.TracedException;
import tools.refinery.language.web.semantics.metadata.MetadataCreator;
import tools.refinery.store.dse.propagation.PropagationRejectedResult;
import tools.refinery.store.dse.propagation.PropagationResult;
import tools.refinery.store.dse.transition.Rule;
import tools.refinery.store.reasoning.literal.Concreteness;
import tools.refinery.store.reasoning.scope.ScopePropagator;
import tools.refinery.store.reasoning.translator.TranslationException;
Expand Down Expand Up @@ -79,18 +83,31 @@ public SemanticsResult call() {
}
cancellationToken.checkCancelled();
var modelResult = createSemanticsModelResult(semantics);
return createSemanticsResult(modelResult, semantics.getPropagationResult());
return createSemanticsResult(modelResult, semantics.getProblemTrace(), semantics.getPropagationResult());
}


private SemanticsResult getTracedErrorResult(EObject sourceElement, String message) {
var diagnostics = getTracedDiagnostics(sourceElement, null, message);
return getSemanticsResultWithDiagnostics(null, message, diagnostics);
}

private List<FeatureBasedDiagnostic> getTracedDiagnostics(
EObject sourceElement, EStructuralFeature feature, String message) {
if (sourceElement == null || !problem.eResource().equals(sourceElement.eResource())) {
return new SemanticsResult(message);
return List.of();
}
var diagnostic = new FeatureBasedDiagnostic(Diagnostic.ERROR, message, sourceElement, null, 0,
var diagnostic = new FeatureBasedDiagnostic(Diagnostic.ERROR, message, sourceElement, feature, 0,
CheckType.EXPENSIVE, DIAGNOSTIC_ID);
var issues = convertIssues(List.of(diagnostic));
return new SemanticsResult(issues);
return List.of(diagnostic);
}

private SemanticsResult getSemanticsResultWithDiagnostics(
SemanticsModelResult modelResult, String message, List<FeatureBasedDiagnostic> diagnostics) {
if (diagnostics.isEmpty()) {
return new SemanticsResult(modelResult, message);
}
var issues = convertIssues(diagnostics);
return new SemanticsResult(modelResult, issues);
}

private List<ValidationResult.Issue> convertIssues(List<FeatureBasedDiagnostic> diagnostics) {
Expand All @@ -114,26 +131,30 @@ private SemanticsModelResult createSemanticsModelResult(ModelSemantics semantics
return new SemanticsModelResult(nodesMetadata, relationsMetadata, partialInterpretation);
}

private SemanticsResult createSemanticsResult(SemanticsModelResult modelResult,
PropagationResult propagationResult) {
private SemanticsResult createSemanticsResult(
SemanticsModelResult modelResult, ProblemTrace trace, PropagationResult propagationResult) {
if (!(propagationResult instanceof PropagationRejectedResult rejectedResult)) {
return new SemanticsResult(modelResult);
}
var message = rejectedResult.formatMessage();
if (!(rejectedResult.reason() instanceof ScopePropagator)) {
return new SemanticsResult(modelResult, message);
}
var diagnostics = new ArrayList<FeatureBasedDiagnostic>();
for (var statement : problem.getStatements()) {
if (statement instanceof ScopeDeclaration scopeDeclaration) {
diagnostics.add(new FeatureBasedDiagnostic(Diagnostic.ERROR, message, scopeDeclaration, null, 0,
CheckType.EXPENSIVE, DIAGNOSTIC_ID));
}
}
if (diagnostics.isEmpty()) {
return new SemanticsResult(modelResult, message);
}
var issues = convertIssues(diagnostics);
return new SemanticsResult(modelResult, issues);
List<FeatureBasedDiagnostic> diagnostics = switch (rejectedResult.reason()) {
case ScopePropagator ignored -> getScopePropagatorDiagnostics(message);
case Rule rule -> getRuleDiagnostics(rule, trace, message);
default -> List.of();
};
return getSemanticsResultWithDiagnostics(modelResult, message, diagnostics);
}

private List<FeatureBasedDiagnostic> getScopePropagatorDiagnostics(String message) {
return problem.getStatements().stream()
.filter(ScopeDeclaration.class::isInstance)
.map(eObject -> new FeatureBasedDiagnostic(Diagnostic.ERROR, message, eObject, null, 0,
CheckType.EXPENSIVE, DIAGNOSTIC_ID))
.toList();
}

private List<FeatureBasedDiagnostic> getRuleDiagnostics(Rule rule, ProblemTrace trace, String message) {
var ruleDefinition = trace.getRuleDefinition(rule);
return getTracedDiagnostics(ruleDefinition, ProblemPackage.Literals.NAMED_ELEMENT__NAME, message);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ default PropagationBuilder rules(Collection<Rule> propagationRules) {

PropagationBuilder propagator(Propagator propagator);

PropagationBuilder throwOnFatalRejection(boolean throwOnFatalRejection);

@Override
PropagationStoreAdapter build(ModelStore store);
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
*/
package tools.refinery.store.dse.propagation;

public record PropagationRejectedResult(Object reason, String message) implements PropagationResult {
public record PropagationRejectedResult(Object reason, String message, boolean fatal) implements PropagationResult {
public PropagationRejectedResult(Object reason, String message) {
this(reason, message, false);
}

@Override
public PropagationResult andThen(PropagationResult next) {
return this;
Expand All @@ -18,7 +22,7 @@ public boolean isRejected() {

@Override
public void throwIfRejected() {
throw new IllegalArgumentException(formatMessage());
throw new IllegalStateException(formatMessage());
}

public String formatMessage() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@
*/
package tools.refinery.store.dse.propagation.impl;

import tools.refinery.store.dse.propagation.BoundPropagator;
import tools.refinery.store.dse.propagation.PropagationAdapter;
import tools.refinery.store.dse.propagation.PropagationResult;
import tools.refinery.store.dse.propagation.PropagationStoreAdapter;
import tools.refinery.store.dse.propagation.*;
import tools.refinery.store.model.Model;

class PropagationAdapterImpl implements PropagationAdapter {
Expand All @@ -35,6 +32,11 @@ public PropagationResult propagate() {
lastResult = propagateOne();
result = result.andThen(lastResult);
} while (lastResult.isChanged());
if (lastResult instanceof PropagationRejectedResult rejectedResult &&
rejectedResult.fatal() &&
storeAdapter.isThrowOnFatalRejection()) {
rejectedResult.throwIfRejected();
}
return result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public class PropagationBuilderImpl extends AbstractModelAdapterBuilder<Propagat
implements PropagationBuilder {
private final Set<Rule> propagationRules = new LinkedHashSet<>();
private final Deque<Propagator> propagators = new ArrayDeque<>();
private boolean throwOnFatalRejection = true;

@Override
public PropagationBuilder rule(Rule propagationRule) {
Expand All @@ -35,6 +36,12 @@ public PropagationBuilder propagator(Propagator propagator) {
return this;
}

@Override
public PropagationBuilder throwOnFatalRejection(boolean throwOnFatalRejection) {
this.throwOnFatalRejection = throwOnFatalRejection;
return this;
}

@Override
protected void doConfigure(ModelStoreBuilder storeBuilder) {
super.doConfigure(storeBuilder);
Expand All @@ -48,6 +55,6 @@ protected void doConfigure(ModelStoreBuilder storeBuilder) {

@Override
protected PropagationStoreAdapter doBuild(ModelStore store) {
return new PropagationStoreAdapterImpl(store, List.copyOf(propagators));
return new PropagationStoreAdapterImpl(store, List.copyOf(propagators), throwOnFatalRejection);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,17 @@

import java.util.List;

// This should not be a record, because we don't want auto-generated {@code equals} and {@code hashCode} methods.
@SuppressWarnings("ClassCanBeRecord")
class PropagationStoreAdapterImpl implements PropagationStoreAdapter {
private final ModelStore store;
private final List<Propagator> propagators;
private final boolean throwOnFatalRejection;

PropagationStoreAdapterImpl(ModelStore store, List<Propagator> propagators) {
PropagationStoreAdapterImpl(ModelStore store, List<Propagator> propagators, boolean throwOnFatalRejection) {
this.store = store;
this.propagators = propagators;
this.throwOnFatalRejection = throwOnFatalRejection;
}

@Override
Expand All @@ -35,4 +39,8 @@ public PropagationAdapter createModelAdapter(Model model) {
List<Propagator> getPropagators() {
return propagators;
}

boolean isThrowOnFatalRejection() {
return throwOnFatalRejection;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ public void afterRestore() {

public PropagationResult fireAll() {
if (!firedActivations.isEmpty()) {
throw new IllegalStateException("Stuck propagation rule '%s'.".formatted(rule.getName()));
return new PropagationRejectedResult(rule, "Propagation rule '%s' got stuck.".formatted(rule.getName()),
true);
}
if (resultSet.size() == 0) {
return PropagationResult.UNCHANGED;
Expand Down

0 comments on commit b52a4b0

Please sign in to comment.