Opened 10 years ago
Closed 6 years ago
#7795 closed defect (fixed)
MPolynomialRing segfaults when getting high exponents
Reported by: | SimonKing | Owned by: | malb |
---|---|---|---|
Priority: | major | Milestone: | sage-duplicate/invalid/wontfix |
Component: | commutative algebra | Keywords: | MPolynomialRing_libsingular segfault with high exponents |
Cc: | Merged in: | ||
Authors: | Reviewers: | Charles Bouillaguet | |
Report Upstream: | Completely fixed; Fix reported upstream | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
In the following example, a segfault occurs. Shouldn't an error be raised instead?
sage: F.<a> = FiniteField(3) sage: P.<T,z> = PolynomialRing(F) sage: type(P) <type 'sage.rings.polynomial.multi_polynomial_libsingular.MPolynomialRing_libsingular'> sage: z^(3^10) z^59049 sage: z^(3^11) z^177147 sage: (z^3 + T*z)^(3^4) z^243 + T^81*z^81 sage: (z^3 + T*z)^(3^7) z^6561 + T^2187*z^2187 sage: (z^3 + T*z)^(3^10) z^177147 + T^59049*z^59049 sage: (z^3 + T*z)^(3^15) /home/king/SAGE/sage-4.3/local/bin/sage-sage: line 206: 20938 Segmentation fault sage-ipython "$@" -i
Change History (22)
comment:1 Changed 10 years ago by
- Milestone set to sage-4.5.1
- Status changed from new to needs_review
comment:2 Changed 10 years ago by
comment:3 Changed 10 years ago by
- Status changed from needs_review to needs_work
- Work issues set to Fix doctests
I did sage -tp 2 devel/sage
and obtained
The following tests failed: sage -t devel/sage/sage/rings/polynomial/multi_polynomial_libsingular.pyx # 1 doctests failed sage -t devel/sage/sage/rings/polynomial/laurent_polynomial_ring.py # 89 doctests failed sage -t devel/sage/sage/rings/polynomial/laurent_polynomial.pyx # 123 doctests failed sage -t devel/sage/sage/combinat/kazhdan_lusztig.py # 5 doctests failed sage -t devel/sage/sage/algebras/iwahori_hecke_algebra.py # 17 doctests failed
At least in one case, it seems that in some places one needed to add further arguments to ring constructors.
comment:4 Changed 10 years ago by
Fixes all but one doctest failure.
comment:5 Changed 10 years ago by
- Status changed from needs_work to needs_review
comment:6 Changed 10 years ago by
Some random timings:
Without the patch
sage: R.<x,y,z> = GF(3)[] sage: p = R.random_element() sage: p -y^2 + x*z - z^2 - z sage: timeit('q=p^100') 125 loops, best of 3: 6.24 ms per loop
With the new patch and the same polynomial, I get:
sage: timeit('q=p^100') 125 loops, best of 3: 2.99 ms per loop
So, this is already good news!
Criticism: The patch still does not provide doctests showing that the problem is fixed. I am now running doctests, and if they pass, I'll try to add a proper doctest via reviewer patch.
comment:7 Changed 10 years ago by
- Status changed from needs_review to needs_work
- Work issues changed from Fix doctests to Fix doctests, fix segfault
The original problem -- namely the segfault when doing (z^3 + T*z)^(3^15)
is not resolved. It segfaults, even with the patch.
comment:8 Changed 8 years ago by
- Report Upstream changed from N/A to Reported upstream. Little or no feedback.
- Status changed from needs_work to needs_info
- I reported this bug upstream as it seems to be an upstream bug
- I deleted the attachment because I moved that code to #12718
comment:9 Changed 8 years ago by
Hans posted a fix here: https://groups.google.com/group/libsingular-devel/msg/39fe65f67ae44ca2?hl=en
comment:10 Changed 8 years ago by
- Report Upstream changed from Reported upstream. Little or no feedback. to Reported upstream. No feedback yet.
comment:11 Changed 7 years ago by
- Report Upstream changed from Reported upstream. No feedback yet. to Completely fixed; Fix reported upstream
- Status changed from needs_info to needs_review
Apparently, this bug is fixed in sage 5.5. I could run (z^3 + T*z)^(3^15)
and it gives the expected answer (after... 9h of computation of a fast machine !). I thus suggest we close this ticket... I don't know how we could doctest that though without running a test for hours.
comment:12 Changed 7 years ago by
NOTE : it worked on a 64-bit machine.
comment:13 follow-up: ↓ 15 Changed 7 years ago by
- Reviewers set to Charles Bouillaguet
- Status changed from needs_review to positive_review
This cannot possibly be doctested, so we might as well close this.
comment:14 Changed 7 years ago by
- Milestone changed from sage-5.6 to sage-duplicate/invalid/wontfix
- Work issues Fix doctests, fix segfault deleted
In this case, please set the milestone to sage-duplicate/invalid/wontfix.
comment:15 in reply to: ↑ 13 ; follow-ups: ↓ 16 ↓ 19 Changed 7 years ago by
Replying to Bouillaguet:
This cannot possibly be doctested, so we might as well close this.
I disagree, in two regards.
First of all, if no error is raised and no crash occurs, then apparently the example is different. Could be a 32-bit versus 64-bit problem. Or not? I don't think that the problem I originally reported was 32 bit.
So, one should try to find an example in which a segfault used to occur, but an error is raised now. Can someone please test whether the following segfaults with sage-4.3?
sage: F.<a> = FiniteField(3) sage: P.<T,z> = PolynomialRing(F) sage: (T+z)^(5^15) Traceback (most recent call last): ... OverflowError: Exponent overflow (30517578125).
If that test segfaults in old Sage versions, then it is a valid test (in particular, those types of bugs can be tested against).
So, who has old Sage versions hanging around?
comment:16 in reply to: ↑ 15 Changed 7 years ago by
Replying to SimonKing:
So, who has old Sage versions hanging around?
Unfortunately, with sage-4.7.beta1, the error is already raised (i.e., no segfault).
comment:17 Changed 7 years ago by
I am now building sage-3.4.1, from which I found the sources somewhere on my machines...
comment:18 Changed 7 years ago by
- Status changed from positive_review to needs_info
comment:19 in reply to: ↑ 15 Changed 7 years ago by
Replying to SimonKing:
First of all, if no error is raised and no crash occurs, then apparently the example is different.
I don't think so. My understanding is that the problem has been fixed inside [lib]singular. Testing this particular example is difficult because the computation takes many hours. It used to crash, but it now works.
comment:20 Changed 6 years ago by
- Status changed from needs_info to needs_review
comment:21 Changed 6 years ago by
- Status changed from needs_review to positive_review
comment:22 Changed 6 years ago by
- Resolution set to fixed
- Status changed from positive_review to closed
The patch does not provide tests for the new functionality. Can these be added, please?
Anyway. The patch applies, sage builds and starts. I'll now do
sage -testall
. Then I'll try some timings involving exponentiation and multiplication. And, unless Martin is quicker, I'll try to add doc tests.