[PATCH] fix bugs in simplifyExp

7 views
Skip to first unread message

Qian Yun

unread,
Dec 21, 2023, 5:19:05 AM12/21/23
to fricas-devel
While writing patches for https://github.com/fricas/fricas/issues/149,
I found 2 other bugs in simplifyExp.

First is 'height' needs recalculation because 'simplifyExp' may cancel
kernels.

However, for 'newK', shall we use 'kernel' here to prevent evaluation
or use 'elt' to do evaluation?

Second is to recurse 'simplifyExp' on the first argument of '%power'.

- Qian

diff --git a/src/algebra/manip.spad b/src/algebra/manip.spad
index 68f5422f..873fd5bf 100644
--- a/src/algebra/manip.spad
+++ b/src/algebra/manip.spad
@@ -773,8 +773,8 @@ TranscendentalManipulations(R, F) : Exports ==
Implementation where
exponent := exponent + d * first argument k
else if not is?(k, POWER) then
-- Expand arguments to functions as well ... MCD 23/1/97
- --coef := coef * monomial(1, k, d)
- coef := coef * monomial(1, kernel(operator k, [simplifyExp u
for u in argument k], height k), d)
+ newK : F := kernel(operator k, [simplifyExp u for u in
argument k])
+ coef := coef * monomial(1, retract(newK)@K, d)
coef::F * exp simplifyExp exponent * powersimp(p, lpow)

expandPower f ==
@@ -790,7 +790,7 @@ TranscendentalManipulations(R, F) : Exports ==
Implementation where
lk := select((z1 : K) : Boolean +-> a = first argument z1, rest l)
for k0 in lk repeat
exponent := exponent + degree(p, k0) * second argument k0
- (a ^ simplifyExp exponent) * powersimp(p, setDifference(rest l, lk))
+ (simplifyExp a ^ simplifyExp exponent) * powersimp(p,
setDifference(rest l, lk))

t2t x == sin(x) / cos(x)
c2t x == cos(x) / sin(x)
diff --git a/src/input/bugs2023.input b/src/input/bugs2023.input
index f60c37ed..07008c6a 100644
--- a/src/input/bugs2023.input
+++ b/src/input/bugs2023.input
@@ -114,4 +114,9 @@ testcase "initial coefficients of sparse univariate
power series"

testEquals("st:=[[n,n+42]@Record(k:INT,c:INT) for n in
0..];s:=series(st)$InnerSparseUnivariatePowerSeries(INT);coefficient(s,0)",
"42")

+testcase "simplifyExp"
+
+testEquals("height simplifyExp sin(exp(a)*exp(-a))", "1")
+testEquals("simplifyExp((exp(a)*exp(b))^c)", "(exp(a+b))^c")
+
statistics()

Waldek Hebisch

unread,
Dec 21, 2023, 2:26:02 PM12/21/23
to fricas...@googlegroups.com
On Thu, Dec 21, 2023 at 06:19:01PM +0800, Qian Yun wrote:
> While writing patches for https://github.com/fricas/fricas/issues/149,
> I found 2 other bugs in simplifyExp.
>
> First is 'height' needs recalculation because 'simplifyExp' may cancel
> kernels.
>
> However, for 'newK', shall we use 'kernel' here to prevent evaluation
> or use 'elt' to do evaluation?

I think that for consistency we should use 'kernel'. Namely,
currently transformations try to be independent and 'kernel'
avoids extra transformations. OTOH it is debatable if we
should keep this independence. And there could be cases where
blindly using 'kernel' could produce strange things (say
'exp(0)'). But IMO this is bigger topic.

> Second is to recurse 'simplifyExp' on the first argument of '%power'.

Thanks, please commit.

--
Waldek Hebisch
Reply all
Reply to author
Forward
0 new messages