diff --git a/symja_android_library/matheclipse-core/src/main/java/org/matheclipse/core/builtin/Algebra.java b/symja_android_library/matheclipse-core/src/main/java/org/matheclipse/core/builtin/Algebra.java index cc4af370b8..074803c216 100644 --- a/symja_android_library/matheclipse-core/src/main/java/org/matheclipse/core/builtin/Algebra.java +++ b/symja_android_library/matheclipse-core/src/main/java/org/matheclipse/core/builtin/Algebra.java @@ -1643,8 +1643,12 @@ public IExpr evaluate(final IAST ast, final int argSize, final IExpr[] options, } if (modulus.isZero() && extension == S.None && gaussianIntegers.isFalse()) { IExpr factorization = AlgebraUtil.factor(ast, arg1, eVar, false, true, true, trig, engine); - return factorization.isPresent() ? AlgebraUtil.factorSigns(factorization, engine) - : factorization; + if (factorization.isPresent()) { + factorization = AlgebraUtil.factorSigns(factorization, engine); + // cache what is returned, so that the cache hit above gives the same form + engine.putCache(ast, factorization); + } + return factorization; } if (!modulus.isZero()) { return factorModulus(S.Factor, arg1, eVar, false, modulus, engine); diff --git a/symja_android_library/matheclipse-core/src/main/java/org/matheclipse/core/eval/AlgebraUtil.java b/symja_android_library/matheclipse-core/src/main/java/org/matheclipse/core/eval/AlgebraUtil.java index 4c0385d613..ac2a0c6a37 100644 --- a/symja_android_library/matheclipse-core/src/main/java/org/matheclipse/core/eval/AlgebraUtil.java +++ b/symja_android_library/matheclipse-core/src/main/java/org/matheclipse/core/eval/AlgebraUtil.java @@ -1583,6 +1583,10 @@ private static IExpr[] extractFractionalPower(IExpr expr) { return null; } + /** + * Writes nothing to the engine cache: the built-in Factor caches the result it + * returns itself, and the other callers factor with other settings under the same key. + */ public static IExpr factor(IAST ast, IExpr arg1, VariablesSet eVar, boolean squareFree, boolean withHomogenization, boolean togetherMode, boolean trig, EvalEngine engine) { IExpr expr = arg1; @@ -1599,11 +1603,8 @@ public static IExpr factor(IAST ast, IExpr arg1, VariablesSet eVar, boolean squa IExpr denominator = factorExpr(F.Factor(fractionParts[1]), fractionParts[1], eVar, squareFree, withHomogenization, trig, engine); if (numerator.isPresent() && denominator.isPresent()) { - IExpr temp = engine.evaluate(F.Divide(numerator, denominator)); - engine.putCache(ast, temp); - return temp; + return engine.evaluate(F.Divide(numerator, denominator)); } - engine.putCache(ast, F.NIL); } catch (JASConversionException e) { // } @@ -1613,7 +1614,6 @@ public static IExpr factor(IAST ast, IExpr arg1, VariablesSet eVar, boolean squa } try { IExpr temp = factorExpr(ast, expr, eVar, squareFree, withHomogenization, trig, engine); - engine.putCache(ast, temp); if (temp.isPresent()) { return temp; } diff --git a/symja_android_library/matheclipse-core/src/test/java/org/matheclipse/core/system/AlgebraTest.java b/symja_android_library/matheclipse-core/src/test/java/org/matheclipse/core/system/AlgebraTest.java index 5ac7ca5936..ce0d527c0c 100644 --- a/symja_android_library/matheclipse-core/src/test/java/org/matheclipse/core/system/AlgebraTest.java +++ b/symja_android_library/matheclipse-core/src/test/java/org/matheclipse/core/system/AlgebraTest.java @@ -717,4 +717,27 @@ public void testFactorIssue1557() { "(1+x)*z"); } + /** + * A cache hit gives what the first Factor of the same argument returned, and a Simplify which + * factors the argument internally does not change what a later Factor returns. + */ + @Test + public void testFactorIssue1570() { + check("{Factor(6/(2-x)), Factor(6/(2-x))}", // + "{-6/(-2+x),-6/(-2+x)}"); + check("{Factor((-a+b)^3), Factor((-a+b)^3)}", // + "{-(a-b)^3,-(a-b)^3}"); + check("{Factor((x+2)/(x-5)+3==6/(2-x)), Factor((x+2)/(x-5)+3==6/(2-x))}", // + "{(-13+4*x)/(-5+x)==-6/(-2+x),(-13+4*x)/(-5+x)==-6/(-2+x)}"); + // TrigSimplifyFu, which Simplify and FullSimplify run, factors the argument first + check("Last({TrigSimplifyFu(6/(2-x)), Factor(6/(2-x))})", // + "-6/(-2+x)"); + check("Last({Simplify(Cos(y)^2/(2-x)), Factor(Cos(y)^2/(2-x))})", // + "-Cos(y)^2/(-2+x)"); + check("Last({FullSimplify((-a+b)^3*Cos(y)), Factor((-a+b)^3*Cos(y))})", // + "-(a-b)^3*Cos(y)"); + check("Last({Simplify(6/(2-x)+Cos(y)), Factor(6/(2-x)+Cos(y))})", // + "(-6-2*Cos(y)+x*Cos(y))/(-2+x)"); + } + }