sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 481 forks source link

Polyring_FpT_coerce cannot handle polynomial_modn_dense_ntl #15954

Open rwst opened 10 years ago

rwst commented 10 years ago
sage: R.<x> = PolynomialRing(Integers(101), implementation='NTL')
sage: 1/x
---------------------------------------------------------------------------
NotImplementedError                       Traceback (most recent call last)
<ipython-input-2-3e814ccafd34> in <module>()
----> 1 Integer(1)/x

/usr/local/src/sage-config/src/sage/rings/integer.pyx in sage.rings.integer.Integer.__div__ (build/cythonized/sage/rings/integer.c:13187)()
   1874             return y
   1875 
-> 1876         return coercion_model.bin_op(left, right, operator.div)
   1877 
   1878     cpdef _div_(self, right):

/usr/local/src/sage-config/src/sage/structure/coerce.pyx in sage.structure.coerce.CoercionModel_cache_maps.bin_op (build/cythonized/sage/structure/coerce.c:9515)()
   1079         try:
   1080             xy = self.canonical_coercion(x,y)
-> 1081             return PyObject_CallObject(op, xy)
   1082         except TypeError as err:
   1083             if xy is not None:

/usr/local/src/sage-config/src/sage/rings/polynomial/polynomial_element.pyx in sage.rings.polynomial.polynomial_element.Polynomial.__div__ (build/cythonized/sage/rings/polynomial/polynomial_element.c:23976)()
   2121 
   2122     def __div__(left, right):
-> 2123         return PyNumber_TrueDivide(left, right)
   2124 
   2125     def __pow__(left, right, modulus):

/usr/local/src/sage-config/src/sage/rings/polynomial/polynomial_element.pyx in sage.rings.polynomial.polynomial_element.Polynomial.__truediv__ (build/cythonized/sage/rings/polynomial/polynomial_element.c:23681)()
   2103         # Same parents => bypass coercion
   2104         if have_same_parent(left, right):
-> 2105             return (<Element>left)._div_(right)
   2106 
   2107         # Try division of polynomial by a scalar

/usr/local/src/sage-config/src/sage/structure/element.pyx in sage.structure.element.RingElement._div_ (build/cythonized/sage/structure/element.c:17628)()
   2476         except AttributeError:
   2477             raise bin_op_exception('/', self, other)
-> 2478         return frac(self, other)
   2479 
   2480     def __invert__(self):

/usr/local/src/sage-config/src/sage/structure/parent.pyx in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9433)()
    917                 return mor._call_(x)
    918             else:
--> 919                 return mor._call_with_args(x, args, kwds)
    920 
    921         raise TypeError("No conversion defined from %s to %s"%(R, self))

/usr/local/src/sage-config/src/sage/rings/fraction_field_FpT.pyx in sage.rings.fraction_field_FpT.Polyring_FpT_coerce._call_with_args (build/cythonized/sage/rings/fraction_field_FpT.cpp:12573)()
   1132             x = <Polynomial_zmod_flint?> _x
   1133         except TypeError:
-> 1134             raise NotImplementedError('Fraction fields not implemented for this type.')
   1135         cdef FpTElement ans = <FpTElement>FpTElement.__new__(FpTElement)
   1136         ans._parent = self.codomain()

NotImplementedError: Fraction fields not implemented for this type.

This makes NTL modpolys unusable in fractions.

Component: coercion

Author: Ralf Stephan

Branch/Commit: u/rws/polyring_fpt_coerce_cannot_handle_polynomial_modn_dense_ntl @ ded2b6d

Reviewer: Karl-Dieter Crisman

Issue created by migration from https://trac.sagemath.org/ticket/15954

rwst commented 10 years ago
comment:2

The error happens when (in Polyring_FpT_coerce._call_with_args) the argument is converted to Polynomial_zmod_flint. As the whole file fraction_field_FpT.pyx is using flint, must we implement it completely via ntl, or does it suffice to fix that single conversion?

rwst commented 10 years ago

Branch: u/rws/polyring_fpt_coerce_cannot_handle_polynomial_modn_dense_ntl

rwst commented 10 years ago

Commit: 7d0e789

rwst commented 10 years ago
comment:4

As no conversion seems possible, a better error message is displayed.


New commits:

7d0e78915954: replace TypeError with more informative message
rwst commented 10 years ago

Author: Ralf Stephan

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

0c634a9Merge branch 'develop' into t/15954/polyring_fpt_coerce_cannot_handle_polynomial_modn_dense_ntl
8d446c115954: add doctest
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from 7d0e789 to 8d446c1

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from 8d446c1 to ded2b6d

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

ded2b6dMerge branch 'develop' into t/15954/polyring_fpt_coerce_cannot_handle_polynomial_modn_dense_ntl
kcrisman commented 9 years ago

Reviewer: Karl-Dieter Crisman

kcrisman commented 9 years ago
comment:8

I like this in general but it's really more than just the fractions, it's when you make the field, so it should also show up here, or maybe even when you make the fraction field, I don't know?

sage: R.<x> = PolynomialRing(Integers(101), implementation='ntl')
sage: R.fraction_field()
Fraction Field of Univariate Polynomial Ring in x over Ring of integers modulo 101 (using NTL)
sage: K = R.fraction_field()
sage: K
Fraction Field of Univariate Polynomial Ring in x over Ring of integers modulo 101 (using NTL)
sage: K(1)
1
sage: K(2)
2
sage: K(x)
TypeError: Cannot convert sage.rings.polynomial.polynomial_modn_dense_ntl.Polynomial_dense_mod_p to sage.rings.polynomial.polynomial_zmod_flint.Polynomial_zmod_flint
rwst commented 6 years ago
comment:9

The issue was resolved independently. In 8.2.beta2 we now get:

sage: R.<x> = PolynomialRing(Integers(101), implementation='NTL')
sage: 1/x
...
NotImplementedError: Fraction fields not implemented for this type.
jdemeyer commented 6 years ago
comment:10

I get a failure already on the first line:

sage: R.<x> = PolynomialRing(Integers(101), implementation='ntl')
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-2-d6c1b40b741c> in <module>()
----> 1 R = PolynomialRing(Integers(Integer(101)), implementation='ntl', names=('x',)); (x,) = R._first_ngens(1)

/usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/rings/polynomial/polynomial_ring_constructor.pyc in PolynomialRing(base_ring, *args, **kwds)
    648         return _multi_variate(base_ring, names, **kwds)
    649     else:
--> 650         return _single_variate(base_ring, names, **kwds)
    651 
    652 

/usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/rings/polynomial/polynomial_ring_constructor.pyc in _single_variate(base_ring, name, sparse, implementation, order)
    734         else:
    735             constructor = polynomial_ring.PolynomialRing_commutative
--> 736         implementation_names = constructor._implementation_names(implementation, base_ring, sparse)
    737         implementation = implementation_names[0]
    738 

/usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/rings/polynomial/polynomial_ring.pyc in _implementation_names(cls, implementation, base_ring, sparse)
    504         if names is NotImplemented:
    505             raise ValueError("unknown implementation %r for %s polynomial rings over %r" %
--> 506                     (implementation, "sparse" if sparse else "dense", base_ring))
    507         assert isinstance(names, list)
    508         assert implementation in names

ValueError: unknown implementation 'ntl' for dense polynomial rings over Ring of integers modulo 101
rwst commented 6 years ago
comment:11

They changed the keyword to NTL without matching lower case. With that I get an informative error message.

jdemeyer commented 6 years ago

Description changed:

--- 
+++ 
@@ -1,28 +1,61 @@

-sage: R. = PolynomialRing(Integers(101), implementation='ntl') +sage: R. = PolynomialRing(Integers(101), implementation='NTL') sage: 1/x

-TypeError Traceback (most recent call last) - in () +NotImplementedError Traceback (most recent call last) + in () ----> 1 Integer(1)/x

-/home/ralf/sage/local/lib/python2.7/site-packages/sage/structure/element.so in sage.structure.element.RingElement.div (sage/structure/element.c:15915)() +/usr/local/src/sage-config/src/sage/rings/integer.pyx in sage.rings.integer.Integer.div (build/cythonized/sage/rings/integer.c:13187)()

-/home/ralf/sage/local/lib/python2.7/site-packages/sage/structure/coerce.so in sage.structure.coerce.CoercionModel_cache_maps.bin_op (sage/structure/coerce.c:7514)() +/usr/local/src/sage-config/src/sage/structure/coerce.pyx in sage.structure.coerce.CoercionModel_cache_maps.bin_op (build/cythonized/sage/structure/coerce.c:9515)()

-/home/ralf/sage/local/lib/python2.7/site-packages/sage/structure/coerce.so in sage.structure.coerce.CoercionModel_cache_maps.bin_op (sage/structure/coerce.c:7459)() +/usr/local/src/sage-config/src/sage/rings/polynomial/polynomial_element.pyx in sage.rings.polynomial.polynomial_element.Polynomial.div (build/cythonized/sage/rings/polynomial/polynomial_element.c:23976)()

-/home/ralf/sage/local/lib/python2.7/site-packages/sage/rings/polynomial/polynomial_element.so in sage.rings.polynomial.polynomial_element.Polynomial.div (sage/rings/polynomial/polynomial_element.c:17669)() +/usr/local/src/sage-config/src/sage/rings/polynomial/polynomial_element.pyx in sage.rings.polynomial.polynomial_element.Polynomial.truediv (build/cythonized/sage/rings/polynomial/polynomial_element.c:23681)()

-/home/ralf/sage/local/lib/python2.7/site-packages/sage/structure/element.so in sage.structure.element.RingElement.div (sage/structure/element.c:15897)() +/usr/local/src/sage-config/src/sage/structure/element.pyx in sage.structure.element.RingElement.div (build/cythonized/sage/structure/element.c:17628)()

-/home/ralf/sage/local/lib/python2.7/site-packages/sage/structure/element.so in sage.structure.element.RingElement.div (sage/structure/element.c:16030)() +/usr/local/src/sage-config/src/sage/structure/parent.pyx in sage.structure.parent.Parent.call (build/cythonized/sage/structure/parent.c:9433)()

-/home/ralf/sage/local/lib/python2.7/site-packages/sage/structure/parent.so in sage.structure.parent.Parent.call (sage/structure/parent.c:8852)() +/usr/local/src/sage-config/src/sage/rings/fraction_field_FpT.pyx in sage.rings.fraction_field_FpT.Polyring_FpT_coerce._call_with_args (build/cythonized/sage/rings/fraction_field_FpT.cpp:12573)()

-/home/ralf/sage/local/lib/python2.7/site-packages/sage/rings/fraction_field_FpT.so in sage.rings.fraction_field_FpT.Polyring_FpT_coerce._call_with_args (sage/rings/fraction_field_FpT.cpp:12184)()

-TypeError: Cannot convert sage.rings.polynomial.polynomial_modn_dense_ntl.Polynomial_dense_mod_p to sage.rings.polynomial.polynomial_zmod_flint.Polynomial_zmod_flint +NotImplementedError: Fraction fields not implemented for this type.

 This makes NTL modpolys unusable in fractions.
jdemeyer commented 6 years ago
comment:13

Replying to @rwst:

The issue was resolved independently.

Which issue was "solved"? It still doesn't work.

rwst commented 6 years ago
comment:15

Well I opened the issue and after that decided to be satisfied with a good error. But hijack away...I don't claim ownership.

jdemeyer commented 6 years ago
comment:16

I don't personally care... but changing the error message does not fix the issue.