Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#6051 closed defect (fixed)

[with patch, positive review] Enable Singular's coefficient rings which are not fields

Reported by: malb Owned by: malb
Priority: major Milestone: sage-4.0.2
Component: commutative algebra Keywords:
Cc: Merged in: 4.0.2.alpha0
Authors: Martin Albrecht Reviewers: Kiran Kedlaya
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Singular 3-1-0 supports coefficient rings which are not fields. In particular, it supports ZZ and ZZ/nZZ now. We should support those natively too.

Attachments (3)

singular_3_1_0-rings.patch (244.0 KB) - added by malb 11 years ago.
almost works
trac_6051-rings-against-4.0.1.patch (240.8 KB) - added by ncalexan 11 years ago.
singular_exp_overflow.patch (1.4 KB) - added by malb 11 years ago.

Download all attachments as: .zip

Change History (18)

Changed 11 years ago by malb

almost works

comment:1 Changed 11 years ago by malb

The attached patch enables the Singular coefficient rings natively. It passes doctests except:

The following tests failed:

        sage -t  devel/sage/sage/rings/polynomial/toy_d_basis.py # 1 doctests failed
----------------------------------------------------------------------
Total time for all tests: 1049.8 seconds

which I reported upstream at

http://www.singular.uni-kl.de:8002/trac/ticket/137

comment:2 Changed 11 years ago by kedlaya

I applied this against 4.0 patched by #6034, and it works great. I don't find any other doctest failures.

comment:3 Changed 11 years ago by malb

FYI I pinged upstream again about this blocker.

comment:4 Changed 11 years ago by ncalexan

Against 4.0.1:

ncalexan@sage:~/releases/sage-4.0.2.alpha0/devel/sage-main/sage$ sage -hg import ~/releases/trac_6051-singular-3_1_0-rings.patch 
applying /home/ncalexan/releases/trac_6051-singular-3_1_0-rings.patch
patching file doc/en/reference/polynomial_rings.rst
Hunk #2 FAILED at 13
1 out of 2 hunks FAILED -- saving rejects to file doc/en/reference/polynomial_rings.rst.rej
patching file sage/rings/polynomial/multi_polynomial_ideal.py
Hunk #14 FAILED at 353
Hunk #52 FAILED at 2195
Hunk #53 FAILED at 2219
Hunk #54 FAILED at 2263
Hunk #55 FAILED at 2271
Hunk #57 FAILED at 2381
6 out of 63 hunks FAILED -- saving rejects to file sage/rings/polynomial/multi_polynomial_ideal.py.rej
patching file sage/rings/polynomial/multi_polynomial_libsingular.pyx
Hunk #16 succeeded at 529 with fuzz 1 (offset 0 lines).
Hunk #17 FAILED at 550
Hunk #87 succeeded at 2650 with fuzz 1 (offset 21 lines).
Hunk #90 succeeded at 2711 with fuzz 1 (offset 23 lines).
1 out of 176 hunks FAILED -- saving rejects to file sage/rings/polynomial/multi_polynomial_libsingular.pyx.rej
abort: patch failed to apply

comment:6 in reply to: ↑ 5 ; follow-up: Changed 11 years ago by ncalexan

Replying to malb:

Upstream fixed the issue in:

ftp://www.mathematik.uni-kl.de/pub/Math/Singular/src/3-1-0/Singular-3-1-0-4.tar.gz

I'm release manager for this. I should update your spkg with this new tree? Will you do that for me?

comment:7 in reply to: ↑ 6 Changed 11 years ago by malb

I'm release manager for this. I should update your spkg with this new tree? Will you do that for me?

Nick, you don't have to update the SPKG just because you are release manager. In any case, I'll see if I can update it soon-ish.

Changed 11 years ago by ncalexan

comment:8 Changed 11 years ago by ncalexan

I rebased the patch against 4.0.1 (really what will be 4.0.2.alpha0) and it works up to that one failing doctest. I'd really like to merge this and #6034 for 4.0.2 so if the spkg itself isn't updated to the even newer singular, let's remove the failing doctest.

comment:9 Changed 11 years ago by malb

There are some issue with the new upstream release (computations timing out), which I haven't tracked down yet. I am a bit short on time so I'd suggest not to include this patch in 4.0.2 or to follow the strategy Nick proposed above: just remove the doctest failure.

comment:10 Changed 11 years ago by ncalexan

  • Authors set to Martin Albrecht
  • Merged in set to 4.0.2.alpha1
  • Resolution set to fixed
  • Reviewers set to Kiran Kedlaya
  • Status changed from new to closed
  • Summary changed from [with patch, needs some work] Enable Singular's coefficient rings which are not fields to [with patch, positive review] Enable Singular's coefficient rings which are not fields

Docstring #random-ed, follow up ticket at #6265.

comment:11 Changed 11 years ago by mvngu

Is this really merged in 4.0.2.alpha1? Do you mean 4.0.2.alpha0?

comment:12 Changed 11 years ago by ncalexan

This is confusing, and the first part (multivariate rings) behave differently on 32 and 64 bit machines. Any thoughts, Martin?

sage: P.<x,y,z> = Integers(2^32)[]
sage: P(2^32-1)
-1
sage: P.<x> = Integers(2^32)[]
sage: P(2^32-1)
4294967295

comment:13 Changed 11 years ago by malb

This looks like an upstream bug to me. I reported it at

http://www.singular.uni-kl.de:8002/trac/ticket/138

I will provide a workaround and attach it to this ticket.

Changed 11 years ago by malb

comment:14 Changed 11 years ago by malb

The attached patch fixes the issue on sage.math for me.

comment:15 Changed 11 years ago by mvngu

  • Merged in changed from 4.0.2.alpha1 to 4.0.2.alpha0
Note: See TracTickets for help on using tickets.