Skip to content
This repository has been archived by the owner on Jan 30, 2023. It is now read-only.

Commit

Permalink
trivial opts in basic arithmetic with number field elements
Browse files Browse the repository at this point in the history
Detailed description by Michael Orlitzky:

    The addition to `_reduce_c_()` bails out early if the denominator is
    one because in that case there's nothing to do. The check is cheap
    and avoids several other function calls that do nothing when the
    denominator is one.

    The changes to  `_add_()` and `_sub_()` are identical. We now pull
    out the GCD of the two denominators (which are smallish numbers)
    early, and it eventually cancels out. This would happen anyway when
    we call `_reduce_c_()` before `return`ing, but at that point we
    would be cancelling the same GCD out of the relatively-largish
    product of the denominators. (This also makes it more likely that we
    hit the new "fast return" in `_reduce_c_()`.)
  • Loading branch information
mezzarobba committed Aug 21, 2019
1 parent 213fe0b commit c71c660
Showing 1 changed file with 18 additions and 6 deletions.
24 changes: 18 additions & 6 deletions src/sage/rings/number_field/number_field_element.pyx
Expand Up @@ -2350,6 +2350,8 @@ cdef class NumberFieldElement(FieldElement):
cdef ZZ_c gcd
cdef ZZ_c t1
cdef ZZX_c t2
if ZZ_IsOne(self.__denominator):
return
ZZX_content(t1, self.__numerator)
ZZ_GCD(gcd, t1, self.__denominator)
if ZZ_sign(gcd) != ZZ_sign(self.__denominator):
Expand All @@ -2372,11 +2374,16 @@ cdef class NumberFieldElement(FieldElement):
"""
cdef NumberFieldElement x
cdef NumberFieldElement _right = right
cdef ZZ_c g, q1, q2
x = self._new()
ZZ_mul(x.__denominator, self.__denominator, _right.__denominator)
ZZ_GCD(g, self.__denominator, _right.__denominator)
ZZ_div(q1, self.__denominator, g)
ZZ_div(q2, _right.__denominator, g)
ZZ_mul(x.__denominator, q1, q2)
ZZ_mul(x.__denominator, x.__denominator, g)
cdef ZZX_c t1, t2
ZZX_mul_ZZ(t1, self.__numerator, _right.__denominator)
ZZX_mul_ZZ(t2, _right.__numerator, self.__denominator)
ZZX_mul_ZZ(t1, self.__numerator, q2)
ZZX_mul_ZZ(t2, _right.__numerator, q1)
ZZX_add(x.__numerator, t1, t2)
x._reduce_c_()
return x
Expand All @@ -2391,11 +2398,16 @@ cdef class NumberFieldElement(FieldElement):
"""
cdef NumberFieldElement x
cdef NumberFieldElement _right = right
cdef ZZ_c g, q1, q2
x = self._new()
ZZ_mul(x.__denominator, self.__denominator, _right.__denominator)
ZZ_GCD(g, self.__denominator, _right.__denominator)
ZZ_div(q1, self.__denominator, g)
ZZ_div(q2, _right.__denominator, g)
ZZ_mul(x.__denominator, q1, q2)
ZZ_mul(x.__denominator, x.__denominator, g)
cdef ZZX_c t1, t2
ZZX_mul_ZZ(t1, self.__numerator, _right.__denominator)
ZZX_mul_ZZ(t2, _right.__numerator, self.__denominator)
ZZX_mul_ZZ(t1, self.__numerator, q2)
ZZX_mul_ZZ(t2, _right.__numerator, q1)
ZZX_sub(x.__numerator, t1, t2)
x._reduce_c_()
return x
Expand Down

0 comments on commit c71c660

Please sign in to comment.