[PATCH] fix 'algreduc' in Expression

[Qian Yun]

(1) -> x1 := sqrt(x)/sqrt(y); x2 := (sqrt(x));

(2) -> setSimplifyDenomsFlag true;

(3) -> x1*x2

+-+2 +-+
\|x \|y
(3) ---------
y

Well, this is a oversight in 'algreduc', as illustrated above.
We should use the already simplified 'x1' instead of 'x'.

(Credits to LLM for spotting it. Test case is reverse
engineered by me.)

- Qian

diff --git a/src/algebra/expr.spad b/src/algebra/expr.spad
index 6b9a11b5..c3f96cb4 100644
--- a/src/algebra/expr.spad
+++ b/src/algebra/expr.spad
@@ -201,7 +201,7 @@
if #akl = 1 then
r := akl(1)
simple_root(r) =>
- return root_reduce(x, r)
+ return root_reduce(x1, r)
sas := create()$SingletonAsOrderedSet
for k in akl repeat
q := univariate(x1, k, minPoly k

[Waldek Hebisch]

On Fri, Jun 12, 2026 at 05:30:09PM +0800, Qian Yun wrote:
> (1) -> x1 := sqrt(x)/sqrt(y); x2 := (sqrt(x));
>
> (2) -> setSimplifyDenomsFlag true;
>
> (3) -> x1*x2
>
> +-+2 +-+
> \|x \|y
> (3) ---------
> y
>
> Well, this is a oversight in 'algreduc', as illustrated above.
> We should use the already simplified 'x1' instead of 'x'.

Thanks, please commit.

> diff --git a/src/algebra/expr.spad b/src/algebra/expr.spad
> index 6b9a11b5..c3f96cb4 100644
> --- a/src/algebra/expr.spad
> +++ b/src/algebra/expr.spad
> @@ -201,7 +201,7 @@
> if #akl = 1 then
> r := akl(1)
> simple_root(r) =>
> - return root_reduce(x, r)
> + return root_reduce(x1, r)
> sas := create()$SingletonAsOrderedSet
> for k in akl repeat
> q := univariate(x1, k, minPoly k
>

> --
> You received this message because you are subscribed to the Google Groups "FriCAS - computer algebra system" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to fricas-devel...@googlegroups.com.
> To view this discussion visit https://groups.google.com/d/msgid/fricas-devel/814f4ca9-12ad-4d9f-b300-190f1e1fc52b%40gmail.com.

--
Waldek Hebisch

[Qian Yun]

On 6/12/26 7:48 PM, Waldek Hebisch wrote:
> On Fri, Jun 12, 2026 at 05:30:09PM +0800, Qian Yun wrote:
>> (1) -> x1 := sqrt(x)/sqrt(y); x2 := (sqrt(x));
>>
>> (2) -> setSimplifyDenomsFlag true;
>>
>> (3) -> x1*x2
>>
>> +-+2 +-+
>> \|x \|y
>> (3) ---------
>> y
>>
>> Well, this is a oversight in 'algreduc', as illustrated above.
>> We should use the already simplified 'x1' instead of 'x'.
>
> Thanks, please commit.
>

I'd like to do commit after the openIfCan test is fixed.

Which workaround do you intend to apply?

- Qian