Opened 10 years ago

Closed 5 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 9 years ago by malb

  • Authors set to Martin Albrecht
  • Milestone set to sage-4.5.1
  • Status changed from new to needs_review

comment:2 Changed 9 years ago by SimonKing

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.

comment:3 Changed 9 years ago by SimonKing

  • 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 9 years ago by malb

Fixes all but one doctest failure.

comment:5 Changed 9 years ago by malb

  • Status changed from needs_work to needs_review

comment:6 Changed 9 years ago by SimonKing

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 9 years ago by SimonKing

  • 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 7 years ago by malb

  • Authors Martin Albrecht deleted
  • 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:10 Changed 7 years ago by roed

  • Report Upstream changed from Reported upstream. Little or no feedback. to Reported upstream. No feedback yet.

comment:11 Changed 7 years ago by Bouillaguet

  • 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 Bouillaguet

NOTE : it worked on a 64-bit machine.

comment:13 follow-up: Changed 7 years ago by Bouillaguet

  • 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 jdemeyer

  • 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: Changed 7 years ago by SimonKing

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 SimonKing

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 SimonKing

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 SimonKing

  • Status changed from positive_review to needs_info

comment:19 in reply to: ↑ 15 Changed 7 years ago by Bouillaguet

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 rws

  • Status changed from needs_info to needs_review

comment:21 Changed 6 years ago by mmezzarobba

  • Status changed from needs_review to positive_review

comment:22 Changed 5 years ago by vbraun

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