|
From: Eberhard S. <esc...@ca...> - 2014-12-29 14:51:08
|
Actually, I was just looking at this code as part of my year end cleanup
of my mail.
Thanks Rainer for doing regression testing on it!
Just to remind what it is supposed to fix. The current code yields
1: q1 := 1/(x+3);
1
q1 := -------
x + 3
2: q2 := (x**2+5*x+6)/(x**2+6*x+9);
2
x + 5*x + 6
q2 := --------------
2
x + 6*x + 9
3: q1+q2;
x + 3
-------
x + 3
Obviously the simplification is incomplete. The updated version of addsq
results in 1 as one would expect.
If nobody objects I will commit the fix by tomorrow evening.
Eberhard
On 12/29/2014 01:22 PM, Rainer Schöpf wrote:
> On Sun, 15 Dec 2013 at 15:46 +0100, Eberhard Schruefer proposed a change to
> addsq:
>
> > symbolic procedure addsq(u,v);
> > % U and V are standard quotients.
> > % Value is canonical sum of U and V.
> > if null numr u then v
> > else if null numr v then u
> > else if denr u=1 and denr v=1 then addf(numr u,numr v) ./ 1
> > else begin scalar x,y,z;
> > if null !*exp then <<u := numr u ./ mkprod denr u;
> > v := numr v ./ mkprod denr v>>;
> > if !*lcm then x := gcdf!*(denr u,denr v)
> > else x := gcdf(denr u,denr v);
> > z := canonsq(quotf(denr u,x) ./ quotf(denr v,x));
> > y := addf(multf(denr z,numr u),multf(numr z,numr v));
> > if null y then return nil ./ 1;
> > z := multf(denr u,denr z);
> > if (x := gcdf(y,x)) neq 1 then <<y := quotf(y,x);
> > z := quotf(z,x)>>;
> > if !*gcd then return if x=1 then y ./ z else canonsq(y ./ z);
> > return if (x := gcdf(y,z))=1 then canonsq(y ./ z)
> > else canonsq(quotf(y,x) ./ quotf(z,x))
> > end;
> >
> > However, this slows down calculations a little bit.
>
> I ran the test suite against this change. The run times differ at most by 5%
> (specfn), in some cases the test runs a bit faster after making the change.
>
> The only notable difference is in the assert test log: with the old code, the
> call
>
> addsq(simp 'x, numr simp 'x);
>
> returns something whose structure is not a standard quotient, but with the new
> code it returns
>
> (1 . nil)
>
> Ie. formally a s.q., but with zero denominator.
>
> Any objection to including this change into the core system?
>
> Rainer
|