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

Finish the flint_base submodule, depreciate old roots and factor out more utils #63

Merged
merged 7 commits into from
Aug 19, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/flint/acb.pyx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from flint.utils.typecheck cimport typecheck
from flint.flint_base.flint_base cimport flint_scalar
from flint.flint_base.flint_context cimport getprec

Expand Down
2 changes: 1 addition & 1 deletion src/flint/acb_mat.pyx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from flint.utils.typecheck cimport typecheck
from flint.flint_base.flint_context cimport getprec

from flint.flint_base.flint_base cimport flint_mat

cdef acb_mat_coerce_operands(x, y):
Expand Down
5 changes: 2 additions & 3 deletions src/flint/acb_poly.pyx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from flint.utils.typecheck cimport typecheck
from flint.flint_base.flint_context cimport getprec
# TODO: waiting for fix on the roots method, currently
# globally defined.
# from flint.flint_base.flint_base cimport flint_poly
from flint.flint_base.flint_base cimport flint_poly

cdef acb_poly_coerce_operands(x, y):
if isinstance(y, (int, long, float, complex, fmpz, fmpq, arb, acb, fmpz_poly, fmpq_poly, arb_poly)):
Expand Down
1 change: 1 addition & 0 deletions src/flint/acb_series.pyx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from flint.utils.typecheck cimport typecheck
from flint.flint_base.flint_context cimport getprec, getcap
from flint.flint_base.flint_base cimport flint_series

Expand Down
1 change: 1 addition & 0 deletions src/flint/arb.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ from cpython.version cimport PY_MAJOR_VERSION

from flint.flint_base.flint_context cimport getprec
from flint.flint_base.flint_base cimport flint_scalar
from flint.utils.typecheck cimport typecheck
from flint.utils.conversion cimport chars_from_str, str_from_chars

cdef _str_trunc(s, trunc=0):
Expand Down
1 change: 1 addition & 0 deletions src/flint/arb_mat.pyx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from flint.utils.typecheck cimport typecheck
from flint.flint_base.flint_context cimport getprec
from flint.flint_base.flint_base cimport flint_mat

Expand Down
5 changes: 2 additions & 3 deletions src/flint/arb_poly.pyx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from flint.utils.typecheck cimport typecheck
from flint.flint_base.flint_context cimport getprec
# TODO: waiting for fix on the roots method, currently
# globally defined.
# from flint.flint_base.flint_base cimport flint_poly
from flint.flint_base.flint_base cimport flint_poly

cdef arb_poly_coerce_operands(x, y):
if isinstance(y, (int, long, float, fmpz, fmpq, arb, fmpz_poly, fmpq_poly)):
Expand Down
1 change: 1 addition & 0 deletions src/flint/arb_series.pyx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from flint.utils.typecheck cimport typecheck
from flint.flint_base.flint_context cimport getprec, getcap
from flint.flint_base.flint_base cimport flint_series

Expand Down
1 change: 1 addition & 0 deletions src/flint/arf.pyx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from flint.flint_base.flint_context cimport getprec
from flint.utils.typecheck cimport typecheck
from flint.utils.conversion cimport prec_to_dps

cdef class arf:
Expand Down
6 changes: 2 additions & 4 deletions src/flint/flint_base/flint_base.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@ cdef class flint_elem:
cdef class flint_scalar(flint_elem):
pass

# TODO:
# See .pyx file
# cdef class flint_poly(flint_elem):
# pass
cdef class flint_poly(flint_elem):
pass

cdef class flint_mpoly(flint_elem):
pass
Expand Down
130 changes: 66 additions & 64 deletions src/flint/flint_base/flint_base.pyx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from warnings import warn

from flint.flint_base.flint_context cimport thectx

cdef class flint_elem:
Expand All @@ -10,78 +12,78 @@ cdef class flint_elem:
def __str__(self):
return self.str()


cdef class flint_scalar(flint_elem):
pass

# TODO:
# We cannot include this class until we can import
# acb_poly, so for now we leave this class as a global
# inside pyflint.pyx
#
# cdef class flint_poly(flint_elem):
# """
# Base class for polynomials.
# """

# def __iter__(self):
# cdef long i, n
# n = self.length()
# for i in range(n):
# yield self[i]

# def coeffs(self):
# return list(self)

# def str(self, bint ascending=False):
# """
# Convert to a human-readable string (generic implementation for
# all polynomial types).

# If *ascending* is *True*, the monomials are output from low degree to
# high, otherwise from high to low.
# """
# coeffs = [str(c) for c in self]
# if not coeffs:
# return "0"
# s = []
# coeffs = enumerate(coeffs)
# if not ascending:
# coeffs = reversed(list(coeffs))
# for i, c in coeffs:
# if c == "0":
# continue
# else:
# if c.startswith("-") or (" " in c):
# c = "(" + c + ")"
# if i == 0:
# s.append("%s" % c)
# elif i == 1:
# if c == "1":
# s.append("x")
# else:
# s.append("%s*x" % c)
# else:
# if c == "1":
# s.append("x^%s" % i)
# else:
# s.append("%s*x^%s" % (c, i))
# return " + ".join(s)

# def roots(self, **kwargs):
# """
# Isolates the complex roots of *self*. See :meth:`.acb_poly.roots`
# for details.
# """
# # TODO:
# # To avoid circular imports, we import within the method
# from XXX.XXX.acb_poly import acb_poly
# return acb_poly(self).roots(**kwargs)


cdef class flint_poly(flint_elem):
"""
Base class for polynomials.
"""

def __iter__(self):
cdef long i, n
n = self.length()
for i in range(n):
yield self[i]

def coeffs(self):
return list(self)

def str(self, bint ascending=False):
"""
Convert to a human-readable string (generic implementation for
all polynomial types).

If *ascending* is *True*, the monomials are output from low degree to
high, otherwise from high to low.
"""
coeffs = [str(c) for c in self]
if not coeffs:
return "0"
s = []
coeffs = enumerate(coeffs)
if not ascending:
coeffs = reversed(list(coeffs))
for i, c in coeffs:
if c == "0":
continue
else:
if c.startswith("-") or (" " in c):
c = "(" + c + ")"
if i == 0:
s.append("%s" % c)
elif i == 1:
if c == "1":
s.append("x")
else:
s.append("%s*x" % c)
else:
if c == "1":
s.append("x^%s" % i)
else:
s.append("%s*x^%s" % (c, i))
return " + ".join(s)

def roots(self):
"""
Depreciated function.
GiacomoPope marked this conversation as resolved.
Show resolved Hide resolved

To recover roots of a polynomial, first convert to acb:

acb_poly(input_poly).roots()
"""
warn('This method is deprecated. Please instead use acb_poly(input_poly).roots()', DeprecationWarning)


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warnings are useful only if we want to preserve existing behaviour while giving out the warning. This does not return the roots that would have previously been returned so existing behaviour is not preserved.

I think it is better just to raise an error here rather than give out a warning and implicity return None.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also if giving an error (or not preserving existing behaviour) then it does not really make sense to refer to this as "deprecated": rather it is "unsupported" or "disallowed" or something like that.

Copy link
Contributor Author

@GiacomoPope GiacomoPope Aug 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree, I'll swap this for

raise NotImplementedError('This method has been deprecated. Please instead use acb_poly(input_poly).roots()')

Example:

Jack: python-flint % python3              
Python 3.11.4 (main, Jul 25 2023, 17:07:07) [Clang 14.0.3 (clang-1403.0.22.14.1)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from flint import *
>>> acb_poly([1,2,3]).roots()
[[-0.333333333 +/- 7.53e-10] + [-0.471404521 +/- 7.03e-10]j, [-0.333333333 +/- 7.53e-10] + [0.471404521 +/- 7.03e-10]j]
>>> nmod_poly([1,2,3], 10).roots()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "src/flint/flint_base/flint_base.pyx", line 75, in flint.flint_base.flint_base.flint_poly.roots
    raise NotImplementedError('This method has been deprecated. Please instead use acb_poly(input_poly).roots()')
NotImplementedError: This method has been deprecated. Please instead use acb_poly(input_poly).roots()

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also we should add a CHANGELOG section at the bottom of the README with something like:

CHANGELOG
---------

0.5.0

- gh-63: The `roots` method of `fmpz_poly`, `fmpq_poly` and `nmod_poly` (others?) is no longer supported. Use `acb_roots(p).roots()` to get the old behaviour of returning the roots as `acb`.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the simplest thing to do is simply say it's not supported and point to the acb_poly() then...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fmpz_poly still has roots, but they're complex roots, which is weird. fmpq_poly asks for roots of the numerator, which I believe is of type fmpz_poly, so the only ones currently broken I think are arb_poly and nmod_poly

>>> acb_poly([1,2]).roots()
[-0.500000000000000]
>>> arb_poly([1,2]).roots()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "src/flint/flint_base/flint_base.pyx", line 75, in flint.flint_base.flint_base.flint_poly.roots
    raise NotImplementedError('This method is no longer supported. To recover the complex roots first convert to acb_poly')
NotImplementedError: This method is no longer supported. To recover the complex roots first convert to acb_poly
>>> fmpz_poly([1,2]).roots()
[(-0.500000000000000, 1)]
>>> fmpq_poly([1,2]).roots()
[(-0.500000000000000, 1)]
>>> nmod_poly([1,2], 11).roots()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "src/flint/flint_base/flint_base.pyx", line 75, in flint.flint_base.flint_base.flint_poly.roots
    raise NotImplementedError('This method is no longer supported. To recover the complex roots first convert to acb_poly')
NotImplementedError: This method is no longer supported. To recover the complex roots first convert to acb_poly


cdef class flint_mpoly(flint_elem):
"""
Base class for multivariate polynomials.
"""


cdef class flint_series(flint_elem):
"""
Base class for power series.
Expand Down
1 change: 1 addition & 0 deletions src/flint/fmpq.pyx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from flint.flint_base.flint_base cimport flint_scalar
from flint.utils.typecheck cimport typecheck

cdef any_as_fmpq(obj):
if typecheck(obj, fmpq):
Expand Down
1 change: 1 addition & 0 deletions src/flint/fmpq_mat.pyx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from flint.flint_base.flint_base cimport flint_mat
from flint.utils.typecheck cimport typecheck

cdef any_as_fmpq_mat(obj):
if typecheck(obj, fmpq_mat):
Expand Down
5 changes: 2 additions & 3 deletions src/flint/fmpq_poly.pyx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# TODO: waiting for fix on the roots method, currently
# globally defined.
# from flint.flint_base.flint_base cimport flint_poly
from flint.utils.typecheck cimport typecheck
from flint.flint_base.flint_base cimport flint_poly

cdef any_as_fmpq_poly(obj):
if typecheck(obj, fmpq_poly):
Expand Down
1 change: 1 addition & 0 deletions src/flint/fmpq_series.pyx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from flint.flint_base.flint_base cimport flint_series
from flint.utils.typecheck cimport typecheck

cdef fmpq_series_coerce_operands(x, y):
if isinstance(y, (int, long, fmpz, fmpz_poly, fmpz_series, fmpq, fmpq_poly)):
Expand Down
1 change: 1 addition & 0 deletions src/flint/fmpz.pyx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from cpython.version cimport PY_MAJOR_VERSION

from flint.flint_base.flint_base cimport flint_scalar
from flint.utils.typecheck cimport typecheck
from flint.utils.conversion cimport chars_from_str

cdef inline int fmpz_set_pylong(fmpz_t x, obj):
Expand Down
1 change: 1 addition & 0 deletions src/flint/fmpz_mat.pyx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from flint.flint_base.flint_base cimport flint_mat
from flint.utils.typecheck cimport typecheck

cdef any_as_fmpz_mat(obj):
if typecheck(obj, fmpz_mat):
Expand Down
1 change: 1 addition & 0 deletions src/flint/fmpz_mpoly.pyx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from cpython.version cimport PY_MAJOR_VERSION

from flint.utils.conversion cimport str_from_chars
from flint.utils.typecheck cimport typecheck
from flint.flint_base.flint_base cimport flint_mpoly

cdef any_as_fmpz_mpoly(x):
Expand Down
5 changes: 2 additions & 3 deletions src/flint/fmpz_poly.pyx
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
from cpython.version cimport PY_MAJOR_VERSION

from flint.flint_base.flint_context cimport getprec
# TODO: waiting for fix on the roots method, currently
# globally defined.
# from flint.flint_base.flint_base cimport flint_poly
from flint.flint_base.flint_base cimport flint_poly
from flint.utils.typecheck cimport typecheck

cdef any_as_fmpz_poly(x):
cdef fmpz_poly res
Expand Down
1 change: 1 addition & 0 deletions src/flint/fmpz_series.pyx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from flint.utils.typecheck cimport typecheck
from flint.flint_base.flint_base cimport flint_series

cdef fmpz_series_coerce_operands(x, y):
Expand Down
1 change: 1 addition & 0 deletions src/flint/nmod.pyx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from flint.flint_base.flint_base cimport flint_scalar
from flint.utils.typecheck cimport typecheck

cdef int any_as_nmod(mp_limb_t * val, obj, nmod_t mod) except -1:
cdef int success
Expand Down
1 change: 1 addition & 0 deletions src/flint/nmod_mat.pyx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from flint.utils.conversion cimport matrix_to_str
from flint.utils.typecheck cimport typecheck

cdef any_as_nmod_mat(obj, nmod_t mod):
cdef nmod_mat r
Expand Down
5 changes: 2 additions & 3 deletions src/flint/nmod_poly.pyx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# TODO: waiting for fix on the roots method, currently
# globally defined.
# from flint.flint_base.flint_base cimport flint_poly
from flint.flint_base.flint_base cimport flint_poly
from flint.utils.typecheck cimport typecheck

cdef any_as_nmod_poly(obj, nmod_t mod):
cdef nmod_poly r
Expand Down
66 changes: 0 additions & 66 deletions src/flint/pyflint.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -24,78 +24,12 @@ cdef extern from "Python.h":
double PyComplex_RealAsDouble(PyObject *op)
double PyComplex_ImagAsDouble(PyObject *op)

cdef inline bint typecheck(object ob, object tp):
return PyObject_TypeCheck(ob, <PyTypeObject*>tp)

DEF FMPZ_UNKNOWN = 0
DEF FMPZ_REF = 1
DEF FMPZ_TMP = 2

ctx = thectx

# TODO:
# This should be factored out into flint_base
# but we cannot do this until we can import
# acb_poly to allow the roots() method to remain

from flint.flint_base.flint_base cimport flint_elem

cdef class flint_poly(flint_elem):
"""
Base class for polynomials.
"""

def __iter__(self):
cdef long i, n
n = self.length()
for i in range(n):
yield self[i]

def coeffs(self):
return list(self)

def str(self, bint ascending=False):
"""
Convert to a human-readable string (generic implementation for
all polynomial types).

If *ascending* is *True*, the monomials are output from low degree to
high, otherwise from high to low.
"""
coeffs = [str(c) for c in self]
if not coeffs:
return "0"
s = []
coeffs = enumerate(coeffs)
if not ascending:
coeffs = reversed(list(coeffs))
for i, c in coeffs:
if c == "0":
continue
else:
if c.startswith("-") or (" " in c):
c = "(" + c + ")"
if i == 0:
s.append("%s" % c)
elif i == 1:
if c == "1":
s.append("x")
else:
s.append("%s*x" % c)
else:
if c == "1":
s.append("x^%s" % i)
else:
s.append("%s*x^%s" % (c, i))
return " + ".join(s)

def roots(self, **kwargs):
"""
Isolates the complex roots of *self*. See :meth:`.acb_poly.roots`
for details.
"""
return acb_poly(self).roots(**kwargs)

include "fmpz.pyx"
include "fmpz_poly.pyx"
include "fmpz_mpoly.pyx"
Expand Down
4 changes: 4 additions & 0 deletions src/flint/utils/typecheck.pxd
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
from flint._flint cimport PyTypeObject, PyObject_TypeCheck

cdef inline bint typecheck(object ob, object tp):
return PyObject_TypeCheck(ob, <PyTypeObject*>tp)