Opened 7 years ago

Closed 7 years ago

#15746 closed defect (fixed)

Make MPolynomialRing_libsingular use the correct coercion map for constants

Reported by: tscrim Owned by: tscrim
Priority: major Milestone: sage-6.2
Component: coercion Keywords: polynomials, finite fields
Cc: fwclarke, pbruin, jpfori Merged in:
Authors: Peter Bruin Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: u/pbruin/15746-MPolynomialRing_libsingular (Commits) Commit: ea2ec42c37fdbb2d4a2e7021ce5e28948e29a80f
Dependencies: Stopgaps:

Description

sage: R.<x,y> = GF(7)[]
sage: R(2^31)
---------------------------------------------------------------------------
OverflowError                             Traceback (most recent call last)
<ipython-input-46-6fd7adc32b9b> in <module>()
----> 1 R(Integer(2)**Integer(31))

/home/travis/sage/local/lib/python2.7/site-packages/sage/rings/polynomial/multi_polynomial_libsingular.so in sage.rings.polynomial.multi_polynomial_libsingular.MPolynomialRing_libsingular.__call__ (sage/rings/polynomial/multi_polynomial_libsingular.cpp:6134)()

/home/travis/sage/local/lib/python2.7/site-packages/sage/rings/polynomial/multi_polynomial_libsingular.so in sage.rings.polynomial.multi_polynomial_libsingular.MPolynomialRing_libsingular._coerce_c_impl (sage/rings/polynomial/multi_polynomial_libsingular.cpp:5633)()

OverflowError: Python int too large to convert to C long

Compare with:

sage: GF(7)(2^31)
2

This might be related to other problems posted in #8857 or fixed by #11239.

Change History (8)

comment:1 Changed 7 years ago by tscrim

  • Cc fwclarke pbruin jpfori added

comment:2 Changed 7 years ago by pbruin

  • Component changed from basic arithmetic to coercion

Part of what is going on here is that Singular works with 32-bit integers; compare

$ ./sage -singular
                     SINGULAR                                 /  Development
 A Computer Algebra System for Polynomial Computations       /   version 3-1-5
                                                           0<
 by: W. Decker, G.-M. Greuel, G. Pfister, H. Schoenemann     \   Jul 2012
FB Mathematik der Universitaet, D-67653 Kaiserslautern        \
> 2^31;
// ** int overflow(^), result may be wrong
-2147483648

The particular error in the ticket description would be fixed by letting the coercion (of the integer into the polynomial ring) go via the base field. Actually, the coercion map itself suggests that this should already happen:

sage: f = R.coerce_map_from(ZZ); f
Composite map:
  From: Integer Ring
  To:   Multivariate Polynomial Ring in x, y over Finite Field of size 7
  Defn:   Natural morphism:
          From: Integer Ring
          To:   Finite Field of size 7
        then
          Polynomial base injection morphism:
          From: Finite Field of size 7
          To:   Multivariate Polynomial Ring in x, y over Finite Field of size 7
sage: f(2^31)
2

So the real problem here is that R(2^31) does not call f as it ought to (I think).

comment:3 Changed 7 years ago by pbruin

The bug appears to be fixed by making MPolynomialRing_libsingular a better-behaved parent: add an attribute Element = MPolynomial_libsingular and rename __call__() to _element_constructor_(). Some more things need to be done (implementing _coerce_map_from_(), probably), because running doctests after this change leads to Sage complaining that this class uses the old coercion framework.

Last edited 7 years ago by pbruin (previous) (diff)

comment:4 Changed 7 years ago by pbruin

  • Authors set to Peter Bruin
  • Branch set to u/pbruin/15746-MPolynomialRing_libsingular
  • Commit set to ea2ec42c37fdbb2d4a2e7021ce5e28948e29a80f
  • Status changed from new to needs_review
  • Summary changed from Polynomials over finite fields gives overflow error with large numbers to Make MPolynomialRing_libsingular use the correct coercion map for constants

Converted MPolynomialRing_libsingular to the not-so-new-anymore coercion system; no substantial changes.

comment:5 Changed 7 years ago by tscrim

LGTM, thanks Peter.

comment:6 Changed 7 years ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

comment:7 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:8 Changed 7 years ago by vbraun

  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.