Skip to content

Commit

Permalink
WIP #1009: improve PM for avoiding wrong evaluation in evalHoldPattern()
Browse files Browse the repository at this point in the history
  • Loading branch information
axkr committed Jul 16, 2024
1 parent b48234f commit 8f38bc2
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public IExpr apply(final IExpr arg) {
return temp;
}
for (int i = 0; i < fMatchers.size(); i++) {
temp = fMatchers.get(i).replace(arg, fEngine, false);
temp = fMatchers.get(i).replace(arg, fEngine);
if (temp.isPresent()) {
return temp;
}
Expand Down Expand Up @@ -122,7 +122,7 @@ public IExpr apply(final IExpr arg) {
for (int i = 0; i < fMatchers.size(); i++) {
PatternMatcherList matcher = fMatchers.get(i);
if (matcher != null) {
matcher.replace(arg, fEngine, false);
matcher.replaceEvaled(arg, fEngine);
IAST list = matcher.getReplaceList();
if (list.size() > 1) {
for (int j = 1; j < list.size(); j++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.matheclipse.core.eval.exception.ThrowException;
import org.matheclipse.core.expression.F;
import org.matheclipse.core.expression.S;
import org.matheclipse.core.interfaces.IAST;
import org.matheclipse.core.interfaces.IExpr;
import org.matheclipse.core.interfaces.ISymbol;

Expand Down Expand Up @@ -148,6 +149,8 @@ public boolean equals(Object obj) {
*/
public abstract IExpr eval(final IExpr leftHandSide, EvalEngine engine);

public abstract IAST getAsAST();

/**
* Get the "left-hand-side" of a pattern-matching rule.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,46 @@ public IExpr eval(final IExpr leftHandSide, EvalEngine engine) {
return F.NIL;
}

@Override
public IAST getAsAST() {
ISymbol setSymbol = getSetSymbol();
IAST temp = F.binaryAST2(setSymbol, getLHS(), getRHS());
if (isFlagOn(HOLDPATTERN)) {
return F.HoldPattern(temp);
}
if (isFlagOn(LITERAL)) {
return F.Literal(temp);
}
return temp;
}

/**
* Return <code>Set</code> or <code>SetDelayed</code> symbol.
*
* @return <code>null</code> if no symbol was defined
*/
public ISymbol getSetSymbol() {
if (isFlagOn(SET_DELAYED)) {
return S.SetDelayed;
}
if (isFlagOn(SET)) {
return S.Set;
}
if (isFlagOn(UPSET_DELAYED)) {
return S.UpSetDelayed;
}
if (isFlagOn(UPSET)) {
return S.UpSet;
}
if (isFlagOn(TAGSET_DELAYED)) {
return S.TagSetDelayed;
}
if (isFlagOn(TAGSET)) {
return S.TagSet;
}
return null;
}

/**
* Get the priority of this pattern-matcher. Lower values have higher priorities.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ public boolean checkRHSCondition(EvalEngine engine) {
/** {@inheritDoc} */
@Override
public IExpr eval(final IExpr leftHandSide, EvalEngine engine) {
return replace(leftHandSide, engine, true);
return replaceEvaled(leftHandSide, engine);
}

public static IExpr evalInternal(final IExpr leftHandSide, final IExpr rightHandSide,
Expand All @@ -228,12 +228,12 @@ public static IExpr evalInternal(final IExpr leftHandSide, final IExpr rightHand
return pm.replacePatternMatch(leftHandSide, patternMap, EvalEngine.get(), true);
}

public IExpr replace(final IExpr leftHandSide, EvalEngine engine, boolean evaluate) {
public IExpr replace(final IExpr leftHandSide, EvalEngine engine) {
IPatternMap patternMap = null;
if (isRuleWithoutPatterns()) {
// no patterns found match equally:
if (fLhsPatternExpr.equals(leftHandSide)) {
return replaceEqualMatch(leftHandSide, engine, evaluate);
return replaceEqualMatch(leftHandSide, engine, false);
}
if (!(fLhsPatternExpr.isOrderlessAST() && leftHandSide.isOrderlessAST())) {
if (!(fLhsPatternExpr.isFlatAST() && leftHandSide.isFlatAST())) {
Expand All @@ -250,7 +250,7 @@ public IExpr replace(final IExpr leftHandSide, EvalEngine engine, boolean evalua
patternMap.initPattern();
setLHSExprToMatch(leftHandSide);
if (matchExpr(fLhsPatternExpr, leftHandSide, engine, new StackMatcher(engine))) {
return replacePatternMatch(leftHandSide, patternMap, engine, evaluate);
return replacePatternMatch(leftHandSide, patternMap, engine, false);
}
}

Expand All @@ -261,6 +261,40 @@ public IExpr replace(final IExpr leftHandSide, EvalEngine engine, boolean evalua
return F.NIL;
}

public IExpr replaceEvaled(final IExpr leftHandSide, EvalEngine engine) {
IPatternMap patternMap = null;
if (isRuleWithoutPatterns()) {
// no patterns found match equally:
if (fLhsPatternExpr.equals(leftHandSide)) {
return replaceEqualMatch(leftHandSide, engine, true);
}
if (!(fLhsPatternExpr.isOrderlessAST() && leftHandSide.isOrderlessAST())) {
if (!(fLhsPatternExpr.isFlatAST() && leftHandSide.isFlatAST())) {
return F.NIL;
}
// replaceSubExpressionOrderlessFlat() below implements equals matching for
// special cases, if the AST is Orderless or Flat
}
if (fLhsPatternExpr.size() == leftHandSide.size()) {
return F.NIL;
}
} else {
patternMap = createPatternMap();
patternMap.initPattern();
setLHSExprToMatch(leftHandSide);
if (matchExpr(fLhsPatternExpr, leftHandSide, engine, new StackMatcher(engine))) {
return replacePatternMatch(leftHandSide, patternMap, engine, true);
}
}

if (fLhsPatternExpr.isASTOrAssociation() && leftHandSide.isASTOrAssociation()) {
return replaceSubExpressionOrderlessFlat((IAST) fLhsPatternExpr, (IAST) leftHandSide,
fRightHandSide, engine);
}
return F.NIL;
}


/**
* A match which contains a pattern was found.
*
Expand All @@ -276,12 +310,6 @@ public IExpr replace(final IExpr leftHandSide, EvalEngine engine, boolean evalua
*/
public IExpr replacePatternMatch(final IExpr leftHandSide, IPatternMap patternMap,
EvalEngine engine, boolean evaluate) {
// if (RulesData.showSteps) {
// if (fLhsPatternExpr.head().equals(S.Integrate)) {
// IExpr rhs = fRightHandSide.orElse(S.Null);
// }
// }

if (fReturnResult.isPresent()) {
if (isFlagOn(IPatternMatcher.SET_DELAYED)) {
boolean oldEvalRHSMode = engine.isEvalRHSMode();
Expand All @@ -298,8 +326,10 @@ public IExpr replacePatternMatch(final IExpr leftHandSide, IPatternMap patternMa
return fReturnResult;
}

boolean oldEvalRHSMode = engine.isEvalRHSMode();
engine.pushOptionsStack();
try {
engine.setEvalRHSMode(true);
if (fLhsPatternExpr != null) {
engine.setOptionsPattern(fLhsPatternExpr.topHead(), patternMap);
}
Expand Down Expand Up @@ -332,6 +362,7 @@ public IExpr replacePatternMatch(final IExpr leftHandSide, IPatternMap patternMa
return result;
} finally {
engine.popOptionsStack();
engine.setEvalRHSMode(oldEvalRHSMode);
}
}

Expand Down Expand Up @@ -378,6 +409,7 @@ public IExpr getSubstitutedMatch() {
return fSubstitutedMatch;
}

@Override
public IAST getAsAST() {
ISymbol setSymbol = getSetSymbol();
IAST temp = F.binaryAST2(setSymbol, getLHS(), getRHS());
Expand All @@ -395,6 +427,7 @@ public IAST getAsAST() {
*
* @return <code>null</code> if no symbol was defined
*/
@Override
public ISymbol getSetSymbol() {
if (isFlagOn(SET_DELAYED)) {
return S.SetDelayed;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,12 @@ private static IExpr toJavaMethodRecursive(DecisionTree dn, StringBuilder buf,
for (int i = 0; i < downRules.size(); i++) {
IPatternMatcher pm = downRules.get(i);
if (patternEval) {

buf.append(" // "
+ pm.getLHS().internalJavaString(RulesToDecisionTree.SCP, 1, fn -> null)
+ " :=\n");
buf.append(" // "
+ pm.getRHS().internalJavaString(RulesToDecisionTree.SCP, 1, fn -> null)
+ "\n");
buf.append("result = PatternMatcherAndEvaluator.evalInternal(evalLHS,"
+ toJava(pm.getRHS()) + ", patternIndexMap );\n");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

/** Generate java sources for Symja rule files. */
public class RulesToDecisionTree {
private static SourceCodeProperties p =
static SourceCodeProperties SCP =
SourceCodeProperties.of(false, false, SourceCodeProperties.Prefix.NONE, false);

/**
Expand All @@ -36,7 +36,6 @@ public class RulesToDecisionTree {
+ "import org.matheclipse.core.interfaces.IAST;\n" //
+ "import static org.matheclipse.core.expression.S.*;\n" //
+ "import java.util.Stack;\n" //
+ "import java.util.Stack;\n" //
+ "import org.matheclipse.core.eval.EvalEngine;\n" //
+ "import org.matheclipse.core.expression.F;\n" //
+ "import org.matheclipse.core.generic.GenericPair;\n" //
Expand Down Expand Up @@ -97,9 +96,9 @@ public static void appendSetDelayedToRule(IAST ast, StringBuilder buffer, boolea
if (evalRHS) {
rightHandSide = F.eval(rightHandSide);
}
buffer.append(leftHandSide.internalJavaString(p, 1, x -> null));
buffer.append(leftHandSide.internalJavaString(SCP, 1, x -> null));
buffer.append(",\n ");
buffer.append(rightHandSide.internalJavaString(p, 1, x -> null));
buffer.append(rightHandSide.internalJavaString(SCP, 1, x -> null));
if (createISet && leftHandSide.isFreeOfPatterns() && !leftHandSide.isSymbol()) {
buffer.append(", true");
}
Expand All @@ -125,9 +124,9 @@ public static void appendSetDelayedToMatcher(IAST ast, StringBuilder buffer, boo
if (evalRHS) {
rightHandSide = F.eval(rightHandSide);
}
buffer.append(leftHandSide.internalJavaString(p, 1, x -> null));
buffer.append(leftHandSide.internalJavaString(SCP, 1, x -> null));
buffer.append(",\n ");
buffer.append(rightHandSide.internalJavaString(p, 1, x -> null));
buffer.append(rightHandSide.internalJavaString(SCP, 1, x -> null));
buffer.append(");\n");
}

Expand Down Expand Up @@ -222,12 +221,17 @@ private static void convertToRule(IExpr expr, String rulePostfix, final PrintWri
buffer.append(" IInit(");
buffer.append(symbolName);
if (equalsRuleCounter > 0 || simpleRuleCounter > 0 || list.size() > 1) {
buffer.append(", SIZES),\n");
if (!symbolName.equals("SphericalHarmonicY")) {
buffer.append(", SIZES),\n");
} else {
buffer.append(", SIZES) \n");
}
} else {
buffer.append(", SIZES) \n");
}
}


for (int i = 1; i < list.size(); i++) {
last = i == (list.argSize());
expr = list.get(i);
Expand Down

0 comments on commit 8f38bc2

Please sign in to comment.