[PATCH] a small optimization for Fraction

[Qian Yun]

I think this is valid optimization, it avoids extra allocation and
computation when the gcd is 1.

- Qian

diff --git a/src/algebra/fraction.spad b/src/algebra/fraction.spad
index d9f4754a..b98a3d37 100644
--- a/src/algebra/fraction.spad
+++ b/src/algebra/fraction.spad
@@ -365,6 +365,7 @@ Fraction(S : IntegralDomain) : QuotientFieldCategory
S with
cancelGcd x ==
((x.den) = 1) => x.den
d := gcd(x.num, x.den)
+ one? d => d
xn := x.num exquo d
xn case "failed" =>
error "gcd not gcd in QF cancelGcd (numerator)"

[Qian Yun]

On 9/26/22 18:40, Qian Yun wrote:
> I think this is valid optimization, it avoids extra allocation and
> computation when the gcd is 1.
>
> - Qian
>
> diff --git a/src/algebra/fraction.spad b/src/algebra/fraction.spad
> index d9f4754a..b98a3d37 100644
> --- a/src/algebra/fraction.spad
> +++ b/src/algebra/fraction.spad
> @@ -365,6 +365,7 @@ Fraction(S : IntegralDomain) : QuotientFieldCategory
> S with
>        cancelGcd x ==
>          ((x.den) = 1) => x.den
>          d := gcd(x.num, x.den)
> +        one? d => d
>          xn := x.num exquo d

Also, does it make sense to use "quo" instead of "exquo" here? ^^^

[Waldek Hebisch]

On Mon, Sep 26, 2022 at 06:40:40PM +0800, Qian Yun wrote:
> I think this is valid optimization, it avoids extra allocation and
> computation when the gcd is 1.

Looks OK.

> - Qian
>
> diff --git a/src/algebra/fraction.spad b/src/algebra/fraction.spad
> index d9f4754a..b98a3d37 100644
> --- a/src/algebra/fraction.spad
> +++ b/src/algebra/fraction.spad
> @@ -365,6 +365,7 @@ Fraction(S : IntegralDomain) : QuotientFieldCategory S
> with
> cancelGcd x ==
> ((x.den) = 1) => x.den
> d := gcd(x.num, x.den)
> + one? d => d
> xn := x.num exquo d
> xn case "failed" =>
> error "gcd not gcd in QF cancelGcd (numerator)"

--
Waldek Hebisch

[Waldek Hebisch]

On Tue, Sep 27, 2022 at 07:47:39PM +0800, Qian Yun wrote:
>
> On 9/26/22 18:40, Qian Yun wrote:
> >I think this is valid optimization, it avoids extra allocation and
> >computation when the gcd is 1.
> >
> >- Qian
> >
> >diff --git a/src/algebra/fraction.spad b/src/algebra/fraction.spad
> >index d9f4754a..b98a3d37 100644
> >--- a/src/algebra/fraction.spad
> >+++ b/src/algebra/fraction.spad
> >@@ -365,6 +365,7 @@ Fraction(S : IntegralDomain) : QuotientFieldCategory S
> >with
> >        cancelGcd x ==
> >          ((x.den) = 1) => x.den
> >          d := gcd(x.num, x.den)
> >+        one? d => d
> >          xn := x.num exquo d
>
> Also, does it make sense to use "quo" instead of "exquo" here? ^^^
>

Well,
1) 'quo' needs EuclideanDomain, we have only GcdDomain here
2) Algorithmically 'exquo' should be faster (it has simpler
work to do). In practice, extra allocation done by
'exquo' means that it can be more expensive when calculation
of quotient is very easy.
3) 'exquo' allows extra consistency check. It found a few bugs in
gcd routines, that probably would be harder to find
otherwise.

--
Waldek Hebisch