From 563d74d2bb651f8c3e34ad1a3fa2eace07056481 Mon Sep 17 00:00:00 2001 From: duy Date: Fri, 23 Oct 2026 21:34:04 +0700 Subject: [PATCH] Factor: a cache hit returns what the first call returned AlgebraUtil.factor(IAST ast, ...) put its result into the engine cache under ast before Factor.evaluate applied factorSigns to it, so since 0d84662822 a second Factor of the same argument returned the form before factorSigns: {Factor(6/(2-x)), Factor(6/(2-x))} gave {-6/(-2+x),6/(2-x)}. AlgebraUtil.factor(IExpr, EvalEngine), which TrigSimplifyFu.tr0 calls for Simplify and FullSimplify, and togetherPlus passed F.Factor(e) as that ast, and so wrote the same key with results computed with other settings: after Simplify(6/(2-x)+Cos(y)) the entry was F.NIL, and Factor returned its argument. Factor.evaluate now caches the result it returns, after factorSigns, and AlgebraUtil.factor writes nothing. Factor.evaluate is the only reader of that cache, so the other callers lose nothing. --- .../org/matheclipse/core/builtin/Algebra.java | 8 +++++-- .../matheclipse/core/eval/AlgebraUtil.java | 10 ++++---- .../matheclipse/core/system/AlgebraTest.java | 23 +++++++++++++++++++ 3 files changed, 34 insertions(+), 7 deletions(-) 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)"); + } + }