Opened 5 years ago

Last modified 11 months ago

#15954 needs_info defect

Polyring_FpT_coerce cannot handle polynomial_modn_dense_ntl

Reported by: rws Owned by:
Priority: major Milestone: sage-8.2
Component: coercion Keywords:
Cc: Merged in:
Authors: Ralf Stephan Reviewers: Karl-Dieter Crisman
Report Upstream: N/A Work issues:
Branch: u/rws/polyring_fpt_coerce_cannot_handle_polynomial_modn_dense_ntl (Commits) Commit: ded2b6d096862fb26e5427ab590ec0686a3dd6e2
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

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.

Change History (16)

comment:1 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:2 Changed 5 years ago by rws

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?

comment:3 Changed 5 years ago by rws

  • Branch set to u/rws/polyring_fpt_coerce_cannot_handle_polynomial_modn_dense_ntl

comment:4 Changed 5 years ago by rws

  • Authors set to Ralf Stephan
  • Commit set to 7d0e789a5b4b3d6b5f08aaa8d92f45dd3dbd0331
  • Status changed from new to needs_review

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


New commits:

7d0e78915954: replace TypeError with more informative message

comment:5 Changed 4 years ago by git

  • Commit changed from 7d0e789a5b4b3d6b5f08aaa8d92f45dd3dbd0331 to 8d446c13b45ed337bbe0f8be779230e383675be8

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

comment:6 Changed 4 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:7 Changed 4 years ago by git

  • Commit changed from 8d446c13b45ed337bbe0f8be779230e383675be8 to ded2b6d096862fb26e5427ab590ec0686a3dd6e2

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

comment:8 Changed 4 years ago by kcrisman

  • Reviewers set to Karl-Dieter Crisman
  • Status changed from needs_review to needs_work

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

comment:9 follow-up: Changed 11 months ago by rws

  • Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
  • Status changed from needs_work to positive_review

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.

comment:10 Changed 11 months ago by jdemeyer

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

comment:11 Changed 11 months ago by rws

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

comment:12 Changed 11 months ago by jdemeyer

  • Description modified (diff)

comment:13 in reply to: ↑ 9 Changed 11 months ago by jdemeyer

  • Status changed from positive_review to needs_info

Replying to rws:

The issue was resolved independently.

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

comment:14 Changed 11 months ago by jdemeyer

  • Milestone changed from sage-duplicate/invalid/wontfix to sage-8.2

comment:15 Changed 11 months ago by rws

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

comment:16 Changed 11 months ago by jdemeyer

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

Note: See TracTickets for help on using tickets.