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
- Cc fwclarke pbruin jpfori added
comment:2 Changed 7 years ago by
- Component changed from basic arithmetic to coercion
comment:3 Changed 7 years ago by
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.
comment:4 Changed 7 years ago by
- 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
LGTM, thanks Peter.
comment:6 Changed 7 years ago by
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
comment:7 Changed 7 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:8 Changed 7 years ago by
- Resolution set to fixed
- Status changed from positive_review to closed
Part of what is going on here is that Singular works with 32-bit integers; compare
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:
So the real problem here is that
R(2^31)
does not callf
as it ought to (I think).