Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility with cython 3 #34

Closed
oscarbenjamin opened this issue Dec 14, 2022 · 1 comment · Fixed by #35
Closed

Compatibility with cython 3 #34

oscarbenjamin opened this issue Dec 14, 2022 · 1 comment · Fixed by #35

Comments

@oscarbenjamin
Copy link
Collaborator

Spin off from #29 (comment). Using the latest Cython solves the coverage testing problem but reveals other problems.

Cython 3.0 is not yet released but there are many prereleases and currently 3.0.0a11 is available. There are some significant changes in Cython and some changes are likely needed in python-flint.

Compiling python-flint with cython==3.0.0a11 gives some warnings and also the tests fail:

$ bin/build_inplace.sh && python test/test.py 
setup.py:9: DeprecationWarning: 

  `numpy.distutils` is deprecated since NumPy 1.23.0, as a result
  of the deprecation of `distutils` itself. It will be removed for
  Python >= 3.12. For older Python versions it will remain present.
  It is recommended to use `setuptools < 60.0` for those Python versions.
  For more details, see:
    https://numpy.org/devdocs/reference/distutils_status_migration.html 


  from numpy.distutils.system_info import default_include_dirs, default_lib_dirs
Compiling src/flint/pyflint.pyx because it depends on src/flint/fmpz.pyx.
[1/1] Cythonizing src/flint/pyflint.pyx
warning: src/flint/_flint.pxd:18:0: The 'DEF' statement is deprecated and will be removed in a future Cython version. Consider using global variables, constants, and in-place literals instead. See https://github.com/cython/cython/issues/4310
warning: src/flint/_flint.pxd:19:0: The 'DEF' statement is deprecated and will be removed in a future Cython version. Consider using global variables, constants, and in-place literals instead. See https://github.com/cython/cython/issues/4310
warning: src/flint/_flint.pxd:20:0: The 'DEF' statement is deprecated and will be removed in a future Cython version. Consider using global variables, constants, and in-place literals instead. See https://github.com/cython/cython/issues/4310
warning: src/flint/pyflint.pyx:55:0: The 'DEF' statement is deprecated and will be removed in a future Cython version. Consider using global variables, constants, and in-place literals instead. See https://github.com/cython/cython/issues/4310
warning: src/flint/pyflint.pyx:56:0: The 'DEF' statement is deprecated and will be removed in a future Cython version. Consider using global variables, constants, and in-place literals instead. See https://github.com/cython/cython/issues/4310
warning: src/flint/pyflint.pyx:57:0: The 'DEF' statement is deprecated and will be removed in a future Cython version. Consider using global variables, constants, and in-place literals instead. See https://github.com/cython/cython/issues/4310
running build_ext
building 'flint._flint' extension
INFO: C compiler: x86_64-linux-gnu-gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O2 -Wall -g -fstack-protector-strong -Wformat -Werror=format-security -g -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC

INFO: compile options: '-I/usr/local/include -I/home/oscar/current/sympy/python-flint/cy_env/include -I/usr/local/include/flint -I/home/oscar/current/sympy/python-flint/cy_env/include/flint -I/home/oscar/current/sympy/python-flint/cy_env/include -I/usr/include/python3.8 -c'
INFO: x86_64-linux-gnu-gcc: src/flint/pyflint.c
src/flint/pyflint.c: In function ‘__pyx_pf_5flint_6_flint_14dirichlet_char_4__init__’:
src/flint/pyflint.c:234106:22: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
       if ((__pyx_t_5 > __pyx_t_6)) {
                      ^
src/flint/pyflint.c: At top level:
src/flint/pyflint.c:50954:18: warning: ‘__pyx_f_5flint_6_flint_any_as_fmpz_mpoly’ defined but not used [-Wunused-function]
 static PyObject *__pyx_f_5flint_6_flint_any_as_fmpz_mpoly(CYTHON_UNUSED PyObject *__pyx_v_x) {
                  ^
INFO: x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -Wl,-Bsymbolic-functions -Wl,-z,relro -g -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -DNDEBUG -g -fwrapv -O2 -Wall build/temp.linux-x86_64-3.8/src/flint/pyflint.o -L/home/oscar/current/sympy/python-flint/cy_env/lib -L/usr/local/lib -larb -lflint -o /home/oscar/current/sympy/python-flint/src/flint/_flint.cpython-38-x86_64-linux-gnu.so
test_fmpz...OK
test_fmpz_poly...Traceback (most recent call last):
  File "test/test.py", line 440, in <module>
    sys.stdout.write("test_fmpz_poly..."); test_fmpz_poly(); print("OK")
  File "test/test.py", line 79, in test_fmpz_poly
    assert ztype(5) + Z([1,2,3]) == Z([6,2,3])
TypeError: unsupported operand type(s) for +: 'int' and 'flint._flint.fmpz_poly'

In particular the new warnings as compared to Cython 0.29.x are these ones about DEF:

warning: src/flint/_flint.pxd:18:0: The 'DEF' statement is deprecated and will be removed in a future Cython version. Consider using global variables, constants, and in-place literals instead. See https://github.com/cython/cython/issues/4310

The other problem is the test failure which is due to changes in Cython:
https://cython.readthedocs.io/en/latest/src/userguide/special_methods.html#arithmetic-methods

This means that now every e.g. __add__ method that should be able to mix with non-python-flint types needs to have a corresponding __radd__ method. There are a lot of places where r-methods might need to be added:

$ git grep __add__
src/flint/acb.pyx:    def __add__(s, t):
src/flint/acb_mat.pyx:    def __add__(s, t):
src/flint/acb_poly.pyx:    def __add__(s, t):
src/flint/acb_series.pyx:    def __add__(s, t):
src/flint/arb.pyx:    def __add__(s, t):
src/flint/arb_mat.pyx:    def __add__(s, t):
src/flint/arb_poly.pyx:    def __add__(s, t):
src/flint/arb_series.pyx:    def __add__(s, t):
src/flint/arf.pyx:    def __add__(s, t):
src/flint/fmpq.pyx:    def __add__(s, t):
src/flint/fmpq_mat.pyx:    def __add__(s, t):
src/flint/fmpq_poly.pyx:    def __add__(s, t):
src/flint/fmpq_series.pyx:    def __add__(s, t):
src/flint/fmpz.pyx:    def __add__(s, t):
src/flint/fmpz_mat.pyx:    def __add__(s, t):
src/flint/fmpz_mpoly.pyx:    def __add__(self, other):
src/flint/fmpz_poly.pyx:    def __add__(self, other):
src/flint/fmpz_series.pyx:    def __add__(s, t):
src/flint/nmod.pyx:    def __add__(s, t):
src/flint/nmod_mat.pyx:    def __add__(s, t):
src/flint/nmod_poly.pyx:    def __add__(s, t):

The fix e.g. for fmpz is like:

diff --git a/src/flint/fmpz.pyx b/src/flint/fmpz.pyx
index dd2703b..f3779fc 100644
--- a/src/flint/fmpz.pyx
+++ b/src/flint/fmpz.pyx
@@ -189,6 +189,17 @@ cdef class fmpz(flint_scalar):
         if ttype == FMPZ_TMP: fmpz_clear(tval)
         return u
 
+    def __radd__(s, t):
+        cdef fmpz_struct tval[1]
+        cdef int ttype = FMPZ_UNKNOWN
+        u = NotImplemented
+        ttype = fmpz_set_any_ref(tval, t)
+        if ttype != FMPZ_UNKNOWN:
+            u = fmpz.__new__(fmpz)
+            fmpz_add((<fmpz>u).val, tval, (<fmpz>s).val)
+        if ttype == FMPZ_TMP: fmpz_clear(tval)
+        return u
+
     def __sub__(s, t):
         cdef fmpz_struct sval[1]
         cdef fmpz_struct tval[1]
@@ -205,6 +216,17 @@ cdef class fmpz(flint_scalar):
         if ttype == FMPZ_TMP: fmpz_clear(tval)
         return u
 
+    def __rsub__(s, t):
+        cdef fmpz_struct tval[1]
+        cdef int ttype = FMPZ_UNKNOWN
+        u = NotImplemented
+        ttype = fmpz_set_any_ref(tval, t)
+        if ttype != FMPZ_UNKNOWN:
+            u = fmpz.__new__(fmpz)
+            fmpz_sub((<fmpz>u).val, tval, (<fmpz>s).val)
+        if ttype == FMPZ_TMP: fmpz_clear(tval)
+        return u
+
     def __mul__(s, t):
         cdef fmpz_struct sval[1]
         cdef fmpz_struct tval[1]
@@ -221,6 +243,17 @@ cdef class fmpz(flint_scalar):
         if ttype == FMPZ_TMP: fmpz_clear(tval)
         return u
 
+    def __rmul__(s, t):
+        cdef fmpz_struct tval[1]
+        cdef int ttype = FMPZ_UNKNOWN
+        u = NotImplemented
+        ttype = fmpz_set_any_ref(tval, t)
+        if ttype != FMPZ_UNKNOWN:
+            u = fmpz.__new__(fmpz)
+            fmpz_mul((<fmpz>u).val, tval, (<fmpz>s).val)
+        if ttype == FMPZ_TMP: fmpz_clear(tval)
+        return u
+
     def __floordiv__(s, t):
         cdef fmpz_struct sval[1]
         cdef fmpz_struct tval[1]
@@ -241,6 +274,21 @@ cdef class fmpz(flint_scalar):
         if ttype == FMPZ_TMP: fmpz_clear(tval)
         return u
 
+    def __rfloordiv__(s, t):
+        cdef fmpz_struct tval[1]
+        cdef int ttype = FMPZ_UNKNOWN
+        u = NotImplemented
+        ttype = fmpz_set_any_ref(tval, t)
+        if ttype != FMPZ_UNKNOWN:
+            if fmpz_is_zero((<fmpz>s).val):
+                if ttype == FMPZ_TMP:
+                    fmpz_clear(tval)
+                raise ZeroDivisionError("fmpz division by zero")
+            u = fmpz.__new__(fmpz)
+            fmpz_fdiv_q((<fmpz>u).val, tval, (<fmpz>s).val)
+        if ttype == FMPZ_TMP: fmpz_clear(tval)
+        return u
+
     def __mod__(s, t):
         cdef fmpz_struct sval[1]
         cdef fmpz_struct tval[1]
@@ -261,6 +309,21 @@ cdef class fmpz(flint_scalar):
         if ttype == FMPZ_TMP: fmpz_clear(tval)
         return u
 
+    def __rmod__(s, t):
+        cdef fmpz_struct tval[1]
+        cdef int ttype = FMPZ_UNKNOWN
+        u = NotImplemented
+        ttype = fmpz_set_any_ref(tval, t)
+        if ttype != FMPZ_UNKNOWN:
+            if fmpz_is_zero((<fmpz>s).val):
+                if ttype == FMPZ_TMP:
+                    fmpz_clear(tval)
+                raise ZeroDivisionError("fmpz division by zero")
+            u = fmpz.__new__(fmpz)
+            fmpz_fdiv_r((<fmpz>u).val, tval, (<fmpz>s).val)
+        if ttype == FMPZ_TMP: fmpz_clear(tval)
+        return u
+
     def __divmod__(s, t):
         cdef fmpz_struct sval[1]
         cdef fmpz_struct tval[1]
@@ -283,6 +346,23 @@ cdef class fmpz(flint_scalar):
         if ttype == FMPZ_TMP: fmpz_clear(tval)
         return u
 
+    def __rdivmod__(s, t):
+        cdef fmpz_struct tval[1]
+        cdef int ttype = FMPZ_UNKNOWN
+        u = NotImplemented
+        ttype = fmpz_set_any_ref(tval, t)
+        if ttype != FMPZ_UNKNOWN:
+            if fmpz_is_zero((<fmpz>s).val):
+                if ttype == FMPZ_TMP:
+                    fmpz_clear(tval)
+                raise ZeroDivisionError("fmpz division by zero")
+            u1 = fmpz.__new__(fmpz)
+            u2 = fmpz.__new__(fmpz)
+            fmpz_fdiv_qr((<fmpz>u1).val, (<fmpz>u2).val, tval, (<fmpz>s).val)
+            u = u1, u2
+        if ttype == FMPZ_TMP: fmpz_clear(tval)
+        return u
+
     def __pow__(s, t, m):
         cdef fmpz_struct sval[1]
         cdef int stype = FMPZ_UNKNOWN
@@ -298,6 +378,21 @@ cdef class fmpz(flint_scalar):
         if stype == FMPZ_TMP: fmpz_clear(sval)
         return u
 
+    def __rpow__(s, t, m):
+        cdef fmpz_struct tval[1]
+        cdef int stype = FMPZ_UNKNOWN
+        cdef ulong exp
+        u = NotImplemented
+        if m is not None:
+            raise NotImplementedError("modular exponentiation")
+        ttype = fmpz_set_any_ref(tval, t)
+        if ttype != FMPZ_UNKNOWN:
+            u = fmpz.__new__(fmpz)
+            s_ulong = fmpz_get_ui(s.val)
+            fmpz_pow_ui((<fmpz>u).val, tval, s_ulong)
+        if ttype == FMPZ_TMP: fmpz_clear(tval)
+        return u
+
     def gcd(self, other):
         """
         Returns the greatest common divisor of self and other.
@oscarbenjamin
Copy link
Collaborator Author

In particular the new warnings as compared to Cython 0.29.x are these ones about DEF:

warning: src/flint/_flint.pxd:18:0: The 'DEF' statement is deprecated and will be removed in a future Cython version. Consider using global variables, constants, and in-place literals instead. See https://github.com/cython/cython/issues/4310

The Cython issue says that DEF is not going to be removed soon but will be replaced by something like cdef const int except that hasn't happened yet cython/cython#4310.

Now gh-35 handles all of the reverse dunder methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant