Sage: Ticket #4539: plural wrapper
https://trac.sagemath.org/ticket/4539
<p>
During SD10 in Nancy, Michael Brickenstein and Burcin Erocal worked on making Plural (the non-commutative extension of Singular) accessible from Sage. Burcin and Michael also worked at the Plural wrapper on SD 23.5 in Kaiserslautern. Oleksandr Motsak and Alexander Dreyer continued this at SD 24 in Linz.
</p>
<p>
The patches that resulted from this work are attached.
</p>
<p>
Newest functionality:
</p>
<ul><li>non-commutative rings/polynomials/ideals are fully featured classes now (no deriving from commutative ones)!
</li><li>coercion from basering/Integer types (still needs tests)
</li><li>flag to check degeneracy conditions on init
</li><li>relations for non-commutative rings
</li><li>most relevant functions for rings/polynomials/ideals (mostly adopted from MPolynomialRing_libsingular/MPolynomialRing_libsingular/...) e.g. std/twostd/syzygy_module/lc/lm/lt/monomial operations
</li><li><a class="missing wiki">RingWrap?</a> and <a class="missing wiki">TermOrder?</a> were extended
</li><li>quick and dirty conversion of <a class="missing wiki">RingWrap?</a> to Sage rings (needs some care as the resulting rings may not be unique and therefore may confuse coercion)
</li><li>quotient of a non-commutative ring by a two-sided Groebner basis
</li><li>shortcut to create graded commutative algebras: SCA
</li></ul><p>
Possible topics that need work are:
</p>
<ul><li>put the files in sage/algebra/ ???
</li><li>make sure element does not export functions it doesn't support (e.g. gcd)
</li><li>predefined structures from the library
</li></ul><p>
<strong><span class="underline">Apply</span></strong>
</p>
<ul><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/trac4539_libplural_rel11761.patch" title="Attachment 'trac4539_libplural_rel11761.patch' in Ticket #4539">trac4539_libplural_rel11761.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/trac4539_libplural_rel11761.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/trac4539_pickling_rel10903.patch" title="Attachment 'trac4539_pickling_rel10903.patch' in Ticket #4539">trac4539_pickling_rel10903.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/trac4539_pickling_rel10903.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/trac4539_normal_forms_rel10903.patch" title="Attachment 'trac4539_normal_forms_rel10903.patch' in Ticket #4539">trac4539_normal_forms_rel10903.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/trac4539_normal_forms_rel10903.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/trac4539_fix_docs_rel10903.patch" title="Attachment 'trac4539_fix_docs_rel10903.patch' in Ticket #4539">trac4539_fix_docs_rel10903.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/trac4539_fix_docs_rel10903.patch" title="Download"></a>
</li></ul>en-usSagehttps://trac.sagemath.org/chrome/site/sage_logo_trac_v2.png
https://trac.sagemath.org/ticket/4539
Trac 1.1.6burcinMon, 17 Nov 2008 15:28:49 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>plural_1.patch</em>
</li>
</ul>
<p>
initial wrapper for plural
</p>
TicketburcinMon, 17 Nov 2008 15:29:20 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>plural_2.patch</em>
</li>
</ul>
<p>
better user interface for plural rings
</p>
TicketPolyBoRiThu, 15 Oct 2009 11:20:31 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>letterplace.py</em>
</li>
</ul>
TicketAlexGhitzaSun, 15 Nov 2009 13:22:40 GMTsummary changed
https://trac.sagemath.org/ticket/4539#comment:1
https://trac.sagemath.org/ticket/4539#comment:1
<ul>
<li><strong>summary</strong>
changed from <em>[with patch, needs work] plural wrapper</em> to <em>plural wrapper</em>
</li>
</ul>
TicketsaliolaTue, 17 Nov 2009 16:26:10 GMTcc set
https://trac.sagemath.org/ticket/4539#comment:2
https://trac.sagemath.org/ticket/4539#comment:2
<ul>
<li><strong>cc</strong>
<em>saliola</em> added
</li>
</ul>
TicketburcinWed, 30 Dec 2009 21:49:46 GMTowner changed; upstream set
https://trac.sagemath.org/ticket/4539#comment:3
https://trac.sagemath.org/ticket/4539#comment:3
<ul>
<li><strong>owner</strong>
changed from <em>tbd</em> to <em>burcin</em>
</li>
<li><strong>upstream</strong>
set to <em>N/A</em>
</li>
</ul>
<p>
The letterplace interface in <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/letterplace.py" title="Attachment 'letterplace.py' in Ticket #4539">attachment:letterplace.py</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/letterplace.py" title="Download"></a> is now at <a class="closed ticket" href="https://trac.sagemath.org/ticket/7797" title="enhancement: Full interface to letterplace from singular (closed: fixed)">#7797</a>.
</p>
TicketburcinWed, 14 Jul 2010 16:46:24 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>plural_1.sage-4.4.4.patch</em>
</li>
</ul>
<p>
rebased to 4.4.4
</p>
TicketburcinWed, 14 Jul 2010 16:46:41 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>plural_2.sage-4.4.4.patch</em>
</li>
</ul>
<p>
rebased to 4.4.4
</p>
TicketburcinThu, 15 Jul 2010 09:40:15 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>plural_3.patch</em>
</li>
</ul>
<p>
appy on top of 1 and 2, new classes for plural objects which don't inherit from the commutative ones
</p>
TicketPolyBoRiThu, 15 Jul 2010 21:03:31 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>plural_functions.2.patch</em>
</li>
</ul>
TicketPolyBoRiThu, 15 Jul 2010 21:06:58 GMT
https://trac.sagemath.org/ticket/4539#comment:4
https://trac.sagemath.org/ticket/4539#comment:4
<p>
sorry, for multiple files. Apply patches in this order:
</p>
<p>
plural_1.sage-4.4.4.patch
plural_2.sage-4.4.4.patch
plural_3.patch Download
plural_functions.patch
</p>
TicketPolyBoRiFri, 16 Jul 2010 09:29:56 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>plural_functions.patch</em>
</li>
</ul>
TicketmhansenTue, 20 Jul 2010 00:14:47 GMTcc changed
https://trac.sagemath.org/ticket/4539#comment:5
https://trac.sagemath.org/ticket/4539#comment:5
<ul>
<li><strong>cc</strong>
<em>mhansen</em> added
</li>
</ul>
TicketOleksandrMotsakTue, 20 Jul 2010 14:10:20 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>plural_folded-4.4.4.patch</em>
</li>
</ul>
<p>
i have just folded all the previous patches by Michael & Burcin into plural_folded-4.4.4.patch (should be applied before anything else)
</p>
TicketAlexanderDreyerTue, 20 Jul 2010 14:28:54 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>extending_plural_folded-4.4.4.patch</em>
</li>
</ul>
<p>
Part one Olekandr's work in Linz
</p>
TicketAlexanderDreyerTue, 20 Jul 2010 14:29:15 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>extplural-more</em>
</li>
</ul>
<p>
Doctest fixes by Alexander
</p>
TicketAlexanderDreyerTue, 20 Jul 2010 14:30:35 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>extplural-more.patch</em>
</li>
</ul>
<p>
Doctest fixes by Alexander
</p>
TicketAlexanderDreyerWed, 21 Jul 2010 10:23:52 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>extplural-more2.patch</em>
</li>
</ul>
<p>
noncommunative ring functionality
</p>
TicketAlexanderDreyerWed, 21 Jul 2010 15:47:26 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>extplural-more3.patch</em>
</li>
</ul>
TicketAlexanderDreyerThu, 22 Jul 2010 12:30:26 GMTowner changed
https://trac.sagemath.org/ticket/4539#comment:6
https://trac.sagemath.org/ticket/4539#comment:6
<ul>
<li><strong>owner</strong>
changed from <em>burcin</em> to <em>OleksandrMotsak,AlexanderDreyer</em>
</li>
</ul>
TicketAlexanderDreyerThu, 22 Jul 2010 12:33:11 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>plural-wrapper-2010-07-22.patch</em>
</li>
</ul>
<p>
This folds of the following patches, a crucial subset of the noncommutation fucntionality as well as extensive documentation and doctests
</p>
TicketAlexanderDreyerThu, 22 Jul 2010 12:35:12 GMTcc changed
https://trac.sagemath.org/ticket/4539#comment:7
https://trac.sagemath.org/ticket/4539#comment:7
<ul>
<li><strong>cc</strong>
<em>AlexanderDreyer</em> added
</li>
</ul>
<p>
We have an first release ready for reviewing!
</p>
<p>
Regards, Oleksandr and Alexander
</p>
TicketAlexanderDreyerThu, 22 Jul 2010 12:36:57 GMTcc changed
https://trac.sagemath.org/ticket/4539#comment:8
https://trac.sagemath.org/ticket/4539#comment:8
<ul>
<li><strong>cc</strong>
<em>OleksandrMotsak</em> <em>PolyBoRi</em> added
</li>
</ul>
TicketAlexanderDreyerThu, 22 Jul 2010 12:37:52 GMTstatus changed
https://trac.sagemath.org/ticket/4539#comment:9
https://trac.sagemath.org/ticket/4539#comment:9
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketAlexanderDreyerThu, 22 Jul 2010 12:39:14 GMTcc changed
https://trac.sagemath.org/ticket/4539#comment:10
https://trac.sagemath.org/ticket/4539#comment:10
<ul>
<li><strong>cc</strong>
<em>malb</em> <em>SimonKing</em> added
</li>
</ul>
TicketPolyBoRiThu, 22 Jul 2010 12:41:34 GMT
https://trac.sagemath.org/ticket/4539#comment:11
https://trac.sagemath.org/ticket/4539#comment:11
<p>
wow, that sounds awesome.
You make me really happy.
Can you outline in the ticket description, what the patch actually implements and what not.
</p>
TicketOleksandrMotsakThu, 22 Jul 2010 13:20:22 GMTdescription changed
https://trac.sagemath.org/ticket/4539#comment:12
https://trac.sagemath.org/ticket/4539#comment:12
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4539?action=diff&version=12">diff</a>)
</li>
</ul>
TicketOleksandrMotsakThu, 22 Jul 2010 13:24:25 GMTdescription changed
https://trac.sagemath.org/ticket/4539#comment:13
https://trac.sagemath.org/ticket/4539#comment:13
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4539?action=diff&version=13">diff</a>)
</li>
</ul>
TicketOleksandrMotsakThu, 22 Jul 2010 13:28:55 GMTdescription changed
https://trac.sagemath.org/ticket/4539#comment:14
https://trac.sagemath.org/ticket/4539#comment:14
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4539?action=diff&version=14">diff</a>)
</li>
</ul>
<p>
what is meant in "predefined structures from the library"?
</p>
<p>
Need input: what structures / what library?
</p>
TicketAlexanderDreyerFri, 23 Jul 2010 06:02:31 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>plural-missing-docu.patch</em>
</li>
</ul>
<p>
Fixed some broken doctests
</p>
TicketAlexanderDreyerSat, 24 Jul 2010 07:22:41 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>plural-missing-docu.2.patch</em>
</li>
</ul>
TicketAlexanderDreyerSat, 24 Jul 2010 07:23:50 GMT
https://trac.sagemath.org/ticket/4539#comment:15
https://trac.sagemath.org/ticket/4539#comment:15
<p>
coverage to 100%
</p>
TicketSimonKingSat, 24 Jul 2010 14:59:25 GMT
https://trac.sagemath.org/ticket/4539#comment:16
https://trac.sagemath.org/ticket/4539#comment:16
<p>
How to apply the patches? <em>All</em> and in the given order? Or is one of them a "master patch" that replaces several other patches
</p>
TicketAlexanderDreyerSun, 25 Jul 2010 19:30:07 GMT
https://trac.sagemath.org/ticket/4539#comment:17
https://trac.sagemath.org/ticket/4539#comment:17
<p>
Just the following:
plural-wrapper-2010-07-22.patch
plural-missing-docu.2.patch
</p>
<p>
Regards,
</p>
<blockquote>
<p>
Alexander
</p>
</blockquote>
TicketAlexanderDreyerTue, 27 Jul 2010 13:35:45 GMTdescription changed
https://trac.sagemath.org/ticket/4539#comment:18
https://trac.sagemath.org/ticket/4539#comment:18
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4539?action=diff&version=18">diff</a>)
</li>
</ul>
TicketAlexanderDreyerTue, 27 Jul 2010 13:51:07 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>plural-wrapper-2010-07-27.patch</em>
</li>
</ul>
<p>
Accumulated patch for all patches above for <a class="missing wiki">Singular/Plural?</a>
</p>
TicketAlexanderDreyerTue, 27 Jul 2010 13:52:17 GMTdescription changed
https://trac.sagemath.org/ticket/4539#comment:19
https://trac.sagemath.org/ticket/4539#comment:19
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4539?action=diff&version=19">diff</a>)
</li>
</ul>
TicketAlexanderDreyerTue, 27 Jul 2010 13:52:51 GMTowner changed
https://trac.sagemath.org/ticket/4539#comment:20
https://trac.sagemath.org/ticket/4539#comment:20
<ul>
<li><strong>owner</strong>
changed from <em>OleksandrMotsak,AlexanderDreyer</em> to <em>OleksandrMotsak, AlexanderDreyer</em>
</li>
</ul>
TicketnthieryFri, 01 Oct 2010 07:58:00 GMT
https://trac.sagemath.org/ticket/4539#comment:21
https://trac.sagemath.org/ticket/4539#comment:21
<blockquote>
<p>
Hi!
</p>
</blockquote>
<p>
I have some computations to do with Weyl algebras, and would love to have this cool piece of work at my fingertips! Please keep it up!
</p>
<p>
For the record: I tried to apply those patches to Sage 4.5.2, and got the following rejects:
</p>
<pre class="wiki">zephyr-/opt/sage/devel/sage>hg qimport ~/plural-wrapper-2010-07-27.patch
adding plural-wrapper-2010-07-27.patch to series file
zephyr-/opt/sage/devel/sage>hg qpush
applying plural-wrapper-2010-07-27.patch
patching file sage/libs/singular/function.pyx
Hunk #3 succeeded at 96 with fuzz 2 (offset 0 lines).
Hunk #36 FAILED at 1378
1 out of 38 hunks FAILED -- saving rejects to file sage/libs/singular/function.pyx.rej
patching file sage/libs/singular/singular-cdefs.pxi
Hunk #3 succeeded at 218 with fuzz 2 (offset -1 lines).
patching file sage/rings/ideal_monoid.py
Hunk #1 FAILED at 12
1 out of 1 hunks FAILED -- saving rejects to file sage/rings/ideal_monoid.py.rej
patching file sage/rings/polynomial/term_order.py
Hunk #1 FAILED at 419
1 out of 1 hunks FAILED -- saving rejects to file sage/rings/polynomial/term_order.py.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh plural-wrapper-2010-07-27.patch
</pre><p>
Cheers,
</p>
TicketAlexanderDreyerFri, 01 Oct 2010 12:07:37 GMT
https://trac.sagemath.org/ticket/4539#comment:22
https://trac.sagemath.org/ticket/4539#comment:22
<p>
Hi nthiery,
Meanwhile, Burcin did a rebase? Does it help you?
</p>
<p>
Regards,
</p>
<blockquote>
<p>
Alexander
</p>
</blockquote>
TicketnthieryFri, 01 Oct 2010 12:32:36 GMT
https://trac.sagemath.org/ticket/4539#comment:23
https://trac.sagemath.org/ticket/4539#comment:23
<p>
Yes, it now applies smoothly on 4.5.2, and compiles. Thanks!
</p>
<pre class="wiki">zephyr-~sage-main>sage
----------------------------------------------------------------------
| Sage Version 4.5.2, Release Date: 2010-08-05 |
| Type notebook() for the GUI, and license() for information. |
----------------------------------------------------------------------
sage: F.<x,dx> = FreeAlgebra(QQ,2)
sage: F.g_algebra({dx*x: x*dx+1})
------------------------------------------------------------
Unhandled SIGSEGV: A segmentation fault occurred in Sage.
This probably occurred because a *compiled* component
of Sage has a bug in it (typically accessing invalid memory)
or is not properly wrapped with _sig_on, _sig_off.
You might want to run Sage under gdb with 'sage -gdb' to debug this.
Sage will now terminate (sorry).
------------------------------------------------------------
</pre><p>
Same problem with the example taken from the documentation:
</p>
<pre class="wiki">zephyr-~sage-main>sage
----------------------------------------------------------------------
| Sage Version 4.5.2, Release Date: 2010-08-05 |
| Type notebook() for the GUI, and license() for information. |
----------------------------------------------------------------------
sage: A.<x,y,z>=FreeAlgebra(QQ,3)
sage: G=A.g_algebra({y*x:-x*y})
------------------------------------------------------------
Unhandled SIGSEGV: A segmentation fault occurred in Sage.
This probably occurred because a *compiled* component
of Sage has a bug in it (typically accessing invalid memory)
or is not properly wrapped with _sig_on, _sig_off.
You might want to run Sage under gdb with 'sage -gdb' to debug this.
Sage will now terminate (sorry).
------------------------------------------------------------
</pre><p>
Should I be using 4.5.3 (being downloaded)?
</p>
TicketAlexanderDreyerFri, 01 Oct 2010 12:44:33 GMT
https://trac.sagemath.org/ticket/4539#comment:24
https://trac.sagemath.org/ticket/4539#comment:24
<p>
Did you rebuild? (<code>sage -br</code>)
</p>
TicketnthieryFri, 01 Oct 2010 12:47:13 GMT
https://trac.sagemath.org/ticket/4539#comment:25
https://trac.sagemath.org/ticket/4539#comment:25
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:24" title="Comment 24">AlexanderDreyer</a>:
</p>
<blockquote class="citation">
<p>
Did you rebuild? (<code>sage -br</code>)
</p>
</blockquote>
<p>
Yup.
</p>
TicketAlexanderDreyerFri, 01 Oct 2010 14:13:26 GMT
https://trac.sagemath.org/ticket/4539#comment:26
https://trac.sagemath.org/ticket/4539#comment:26
<p>
I could reproduce the issue: <code>sage -gdb</code> vields the following;
</p>
<pre class="wiki">sage: A.<x,y,z>=FreeAlgebra(QQ,3)
sage: G=A.g_algebra({y*x:-x*y})
Program received signal SIGSEGV, Segmentation fault.
0x00007fffdec7c488 in __pyx_f_4sage_4libs_8singular_8function_call_function (__pyx_v_self=0x459f910, __pyx_v_args=0x4538ab8, __pyx_v_R=0x4676480,
__pyx_optional_args=<value optimized out>) at sage/libs/singular/function.cpp:11969
11969 currRingHdl->data.uring->ref += 1;
</pre>
TicketburcinFri, 01 Oct 2010 14:27:31 GMTstatus changed
https://trac.sagemath.org/ticket/4539#comment:27
https://trac.sagemath.org/ticket/4539#comment:27
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
I didn't have a chance to test the rebased patch, I had to leave right after I finished merging the rejected parts. I just wanted to get it out there in case it worked.
</p>
<p>
I can also reproduce the crash. I'll take a look at it now and try to upload a patch that works.
</p>
TicketburcinFri, 01 Oct 2010 14:32:43 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>plural-wrapper-2010-10-01.patch</em>
</li>
</ul>
<p>
rebased to 4.5.3
</p>
TicketburcinFri, 01 Oct 2010 14:44:23 GMT
https://trac.sagemath.org/ticket/4539#comment:28
https://trac.sagemath.org/ticket/4539#comment:28
<p>
It was indeed careless rebasing. <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/plural-wrapper-2010-10-01.patch" title="Attachment 'plural-wrapper-2010-10-01.patch' in Ticket #4539">attachment:plural-wrapper-2010-10-01.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/plural-wrapper-2010-10-01.patch" title="Download"></a> (patch with same name as before, to hide my shame :) ) seems to work.
</p>
<p>
Nicolas, it would be great if you could help with the review. We are pretty confident with the interface to Singular and low-level code, since, as you can also see from the comments on the ticket, many Singular and Sage developers were involved in writing the code. However, we added many of the noncommutative structures on the spot (in a late night coding sprint) as we needed them. Another pair of eyes checking the mathematical structures and design would be really useful.
</p>
<p>
Though I think we should try to get this patch in as soon as possible. I'm sure quite a few people would be interested in the functionality of Plural. We can always work on providing a better interface later, as the number of users/developers increases.
</p>
TicketnthierySun, 03 Oct 2010 21:06:26 GMT
https://trac.sagemath.org/ticket/4539#comment:29
https://trac.sagemath.org/ticket/4539#comment:29
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:28" title="Comment 28">burcin</a>:
</p>
<blockquote class="citation">
<p>
It was indeed careless rebasing. <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/plural-wrapper-2010-10-01.patch" title="Attachment 'plural-wrapper-2010-10-01.patch' in Ticket #4539">attachment:plural-wrapper-2010-10-01.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/plural-wrapper-2010-10-01.patch" title="Download"></a> (patch with same name as before, to hide my shame :) ) seems to work.
</p>
</blockquote>
<p>
It also does for me so far! Thanks a lot for the quick rebase!
</p>
<blockquote class="citation">
<p>
Nicolas, it would be great if you could help with the review. We are pretty confident with the interface to Singular and low-level code, since, as you can also see from the comments on the ticket, many Singular and Sage developers were involved in writing the code. However, we added many of the noncommutative structures on the spot (in a late night coding sprint) as we needed them. Another pair of eyes checking the mathematical structures and design would be really useful.
</p>
</blockquote>
<p>
I don't want to promise much at this time because I am already (very) late with a couple other reviews. But since the code is going to very useful for my research right now, I can promise to provide feedback for how it feels in practice!
</p>
<blockquote class="citation">
<p>
Though I think we should try to get this patch in as soon as possible. I'm sure quite a few people would be interested in the functionality of Plural. We can always work on providing a better interface later, as the number of users/developers increases.
</p>
</blockquote>
<p>
Sounds good to me!
</p>
TicketnthierySun, 03 Oct 2010 21:06:40 GMTstatus changed
https://trac.sagemath.org/ticket/4539#comment:30
https://trac.sagemath.org/ticket/4539#comment:30
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketnthierySun, 03 Oct 2010 21:08:57 GMT
https://trac.sagemath.org/ticket/4539#comment:31
https://trac.sagemath.org/ticket/4539#comment:31
<p>
Is it possible at this stage to define non commutative polynomial rings over QQ<a class="missing wiki">q?</a>.fraction_field()?
I got an error with what I tried:
</p>
<pre class="wiki">sage: K = QQ['q'].fraction_field()
q = K.gen()
F.<x,y,ex,ey> = FreeAlgebra(K,4)
W = F.g_algebra({ex*x: x*(1+ex),ey*y:y*(1+ey)})
sage: ------------------------------------------------------------
Traceback (most recent call last):
File "<ipython console>", line 1, in <module>
File "/opt/sage-4.5.2/local/lib/python2.6/site-packages/sage/algebras/free_algebra.py", line 547, in g_algebra
return NCPolynomialRing_plural(base_ring, n, ",".join([str(g) for g in self.gens()]), c=cmat, d=dmat, order=order, check=check)
File "plural.pyx", line 223, in sage.rings.polynomial.plural.NCPolynomialRing_plural.__init__ (sage/rings/polynomial/plural.cpp:3772)
File "matrix0.pyx", line 1520, in sage.matrix.matrix0.Matrix.change_ring (sage/matrix/matrix0.c:7670)
File "/opt/sage-4.5.2/local/lib/python2.6/site-packages/sage/matrix/matrix_space.py", line 405, in __call__
return self.matrix(entries, copy=copy, coerce=coerce, rows=rows)
File "/opt/sage-4.5.2/local/lib/python2.6/site-packages/sage/matrix/matrix_space.py", line 1136, in matrix
return self.__matrix_class(self, entries=x, copy=copy, coerce=coerce)
File "matrix_generic_dense.pyx", line 93, in sage.matrix.matrix_generic_dense.Matrix_generic_dense.__init__ (sage/matrix/matrix_generic_dense.c:2321)
File "/opt/sage-4.5.2/local/lib/python2.6/site-packages/sage/rings/polynomial/multi_polynomial_ring.py", line 468, in __call__
c = self.base_ring()(x)
File "parent.pyx", line 859, in sage.structure.parent.Parent.__call__ (sage/structure/parent.c:6407)
File "coerce_maps.pyx", line 82, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (sage/structure/coerce_maps.c:3108)
File "coerce_maps.pyx", line 77, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (sage/structure/coerce_maps.c:3010)
File "/opt/sage-4.5.2/local/lib/python2.6/site-packages/sage/rings/fraction_field.py", line 467, in _element_constructor_
coerce=coerce, reduce = self.is_exact())
File "fraction_field_element.pyx", line 120, in sage.rings.fraction_field_element.FractionFieldElement.__init__ (sage/rings/fraction_field_element.c:1934)
File "parent.pyx", line 859, in sage.structure.parent.Parent.__call__ (sage/structure/parent.c:6407)
File "coerce_maps.pyx", line 82, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (sage/structure/coerce_maps.c:3108)
File "coerce_maps.pyx", line 77, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (sage/structure/coerce_maps.c:3010)
File "/opt/sage-4.5.2/local/lib/python2.6/site-packages/sage/rings/polynomial/polynomial_ring.py", line 313, in _element_constructor_
return C(self, x, check, is_gen, construct=construct, **kwds)
File "/opt/sage-4.5.2/local/lib/python2.6/site-packages/sage/rings/polynomial/polynomial_element_generic.py", line 656, in __init__
x = [QQ(z) for z in x]
File "parent.pyx", line 859, in sage.structure.parent.Parent.__call__ (sage/structure/parent.c:6407)
File "coerce_maps.pyx", line 82, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (sage/structure/coerce_maps.c:3108)
File "coerce_maps.pyx", line 77, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (sage/structure/coerce_maps.c:3010)
File "rational.pyx", line 367, in sage.rings.rational.Rational.__init__ (sage/rings/rational.c:5781)
File "rational.pyx", line 521, in sage.rings.rational.Rational.__set_value (sage/rings/rational.c:7052)
TypeError: Unable to coerce 0 (<class 'sage.algebras.free_algebra_element.FreeAlgebraElement'>) to Rational
</pre><p>
Thanks!
</p>
TicketnthierySun, 03 Oct 2010 21:22:12 GMT
https://trac.sagemath.org/ticket/4539#comment:32
https://trac.sagemath.org/ticket/4539#comment:32
<p>
I started playing with ideals. Currently, one creates an ideal I, and
then when one calls I.std() or I.twostd() to create a new left or
twosided ideal, and actually compute the Groebner basis. What about
the following variants:
</p>
<p>
(A) Take the side decision at the time the ideal is created:
</p>
<pre class="wiki"> sage: I = W.ideal([...], side=...)
</pre><p>
(to be documented in <code></code>W.ideal?<code></code>).
</p>
<p>
With that, <code></code>I<code></code> would be well defined as an ideal; otherwise it is
more a <code></code>collection of polynomials<em> and the name is misleading.
</em></p>
<p>
(B) About computing the Grobner basis:
</p>
<pre class="wiki"> sage: I.groebner_basis()
</pre><p>
or:
</p>
<pre class="wiki"> sage: I.std()
</pre><p>
would compute the groebner basis, store it for later calculations, and
return it as a list of polynomials rather than a new ideal.
</p>
<p>
I haven't actually played with ideals in Sage; so maybe point (B) is
just how things are with commutative ideals in Sage, and consistency
should take precedence.
</p>
<p>
Cheers,
</p>
<blockquote>
<p>
Nicolas
</p>
</blockquote>
TicketSimonKingSun, 03 Oct 2010 21:58:00 GMT
https://trac.sagemath.org/ticket/4539#comment:33
https://trac.sagemath.org/ticket/4539#comment:33
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:32" title="Comment 32">nthiery</a>:
</p>
<blockquote class="citation">
<p>
I started playing with ideals. Currently, one creates an ideal I, and
then when one calls I.std() or I.twostd() to create a new left or
twosided ideal, and actually compute the Groebner basis. What about
the following variants:
</p>
</blockquote>
<p>
Currently, if R is a commutative ring and L is a list of elements of R, one may use the shorthand notation <code>I = R*L</code> or <code>I = L*R</code> to create an ideal. It seems natural to me to extend this to the non-commutative case: <code>R*L</code> for left ideal, <code>L*R</code> for right ideal, and <code>R*L*R</code> for two-sided ideal.
</p>
<p>
How to implement it? Well, on could have a base class for ideals over non-commutative rings (let's call it <code>NCIdeal</code>), and derive from it <code>NCIdeal_left</code>, <code>NCIdeal_right</code>, <code>NCIdeal_twodsided</code>.
</p>
<p>
Then, one has to modify the multiplication method for rings so that sidedness is taken care of (there is a method ideal_class(), that probably needs to accept an optional argument "side"). And of course, the one-sided ideal classes need a multiplication method as well.
</p>
<p>
And then, <code>NCIdeal_twodsided.groebner_basis()</code> would yield a two-sided Gröbner basis, while <code>NCIdeal_left/right.groebner_basis()</code> would only yield a one-sided Gröbner basis, of course unless a two-sided Gröbner basis is requested by using an optional argument.
</p>
TicketnthieryTue, 12 Oct 2010 15:20:52 GMT
https://trac.sagemath.org/ticket/4539#comment:34
https://trac.sagemath.org/ticket/4539#comment:34
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:32" title="Comment 32">nthiery</a>:
</p>
<blockquote class="citation">
<p>
I started playing with ideals. Currently, one creates an ideal I, and
then when one calls I.std() or I.twostd() to create a new left or
twosided ideal, and actually compute the Groebner basis.
</p>
</blockquote>
<p>
By the way: right ideals are not yet handled yet, right? Would it be a lot of work? It's just that the ideals I am currently playing with are right ideals, and I keep mixing myself up when playing with the "dualized" version I had to write in Sage.
</p>
TicketnthieryTue, 12 Oct 2010 15:28:01 GMT
https://trac.sagemath.org/ticket/4539#comment:35
https://trac.sagemath.org/ticket/4539#comment:35
<blockquote>
<p>
Hi Simon!
</p>
</blockquote>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:33" title="Comment 33">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:32" title="Comment 32">nthiery</a>:
</p>
<blockquote class="citation">
<p>
I started playing with ideals. Currently, one creates an ideal I, and
then when one calls I.std() or I.twostd() to create a new left or
twosided ideal, and actually compute the Groebner basis. What about
the following variants:
</p>
</blockquote>
<p>
Currently, if R is a commutative ring and L is a list of elements of R, one may use the shorthand notation <code>I = R*L</code> or <code>I = L*R</code> to create an ideal. It seems natural to me to extend this to the non-commutative case: <code>R*L</code> for left ideal, <code>L*R</code> for right ideal, and <code>R*L*R</code> for two-sided ideal.
</p>
</blockquote>
<p>
+1 for the implementation proposal!
</p>
<p>
I also like that shorthand syntax for interactive usage. However, in
code, I prefer using something more explicit like R.ideal(L,side=...).
Besides, having an R.ideal method also provides with:
</p>
<ul><li>a good place for advertising (it appears in R.<tab>), documenting,
testing the feature and its shorthand
</li><li>an easy way for subclasses of (the class of) R to override this
method without having to worry about overloading/coercion/...
</li></ul><p>
Cheers,
</p>
<blockquote>
<p>
Nicolas
</p>
</blockquote>
TicketSimonKingSun, 13 Mar 2011 10:44:31 GMT
https://trac.sagemath.org/ticket/4539#comment:36
https://trac.sagemath.org/ticket/4539#comment:36
<p>
Apply plural-wrapper-2010-10-01.patch
</p>
<p>
(to the patchbot)
</p>
<p>
If I understand correctly, this patch is the only one, right? So, it would be a good thing to try whether it needs to be rebased again.
</p>
<p>
Currently, I have a high motivation to have the noncommutative stuff (including letterplace!) in libsingular. So, I hope the work on this ticket and on <a class="closed ticket" href="https://trac.sagemath.org/ticket/7797" title="enhancement: Full interface to letterplace from singular (closed: fixed)">#7797</a> can be resumed.
</p>
TicketSimonKingSun, 13 Mar 2011 14:13:21 GMTstatus changed; keywords, work_issues set
https://trac.sagemath.org/ticket/4539#comment:37
https://trac.sagemath.org/ticket/4539#comment:37
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>keywords</strong>
<em>libsingular</em> <em>plural</em> <em>wrapper</em> added
</li>
<li><strong>work_issues</strong>
set to <em>Implement is_integral_domain and probably other small stuff</em>
</li>
</ul>
<p>
Apply trac4539_libplural.patch
</p>
<p>
It turned out that the old patch did not apply, since meanwhile the libsingular options are dealt with in a different way. I have rebased the patch, and also adopted the new option syntax.
</p>
<p>
However, not all is good. Here is one error that I just found:
</p>
<pre class="wiki">sage: A.<x,y,z> = FreeAlgebra(QQ, 3)
sage: H = A.g_algebra({y*x:x*y-z, z*x:x*z+2*x, z*y:y*z-2*y})
sage: H.inject_variables()
Defining x, y, z
sage: I = H.ideal([y^2, x^2, z^2-H.one_element()],coerce=False)
sage: G = vector(I.gens()); G
---------------------------------------------------------------------------
NotImplementedError Traceback (most recent call last)
...
/mnt/local/king/SAGE/sage-4.6.2/local/lib/python2.6/site-packages/sage/rings/ring.so in sage.rings.ring.Ring.is_integral_domain (sage/rings/ring.c:6035)()
NotImplementedError:
</pre><p>
So, there remains work to do!
</p>
TicketSimonKingSun, 13 Mar 2011 14:27:36 GMTstatus changed; work_issues deleted
https://trac.sagemath.org/ticket/4539#comment:38
https://trac.sagemath.org/ticket/4539#comment:38
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>work_issues</strong>
<em>Implement is_integral_domain and probably other small stuff</em> deleted
</li>
</ul>
<p>
The reason of the error was that <code>FreeModule</code> tries (among other things) the method <code>is_integral_domain()</code>. By default, it raises a <code>NotImplementedError</code>, and this is the error we get above.
</p>
<p>
Proposed solution: Catch that <code>NotImplementedError</code> and do as if <code>is_integral_domain()</code> had returned <code>False</code>.
</p>
<p>
I don't know if there will be further problems, but I'll put it back to "needs review".
</p>
TicketSimonKingSun, 13 Mar 2011 18:25:15 GMT
https://trac.sagemath.org/ticket/4539#comment:39
https://trac.sagemath.org/ticket/4539#comment:39
<p>
Apply trac4539_libplural.patch
</p>
TicketSimonKingSun, 13 Mar 2011 18:27:02 GMTstatus changed
https://trac.sagemath.org/ticket/4539#comment:40
https://trac.sagemath.org/ticket/4539#comment:40
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<ol><li>I don't know why the patchbot is trying <em>two</em> patches. It is supposed to use only one of them.
</li></ol><ol start="2"><li>I get numerous doctest failures. Some of them look like severe errors. So, it needs work.
</li></ol>
TicketjhpalmieriWed, 23 Mar 2011 22:16:47 GMT
https://trac.sagemath.org/ticket/4539#comment:41
https://trac.sagemath.org/ticket/4539#comment:41
<p>
It would be great to have this in Sage!
</p>
<p>
I'm seeing some problems with the patch. First, it doesn't apply cleanly to Sage 4.7.alpha1. I haven't tried applying to 4.6.2. I rebased it by hand.
</p>
<p>
Second, the change
</p>
<div class="wiki-code"><div class="code"><pre><span class="gd">- block_name, block_length, _ = re.split(length_pattern,block)
</span><span class="gi">+ block_name, block_length, _ = re.split(pattern, block.strip())
</span></pre></div></div><p>
in term_order.py is problematic, because "pattern" is not defined anywhere. Replacing it by "length_pattern" seems to work.
</p>
<p>
Third, in multipolynomial_ideal.py, <code>_groebner_basis_libsingular</code> is being called with keywords "deg_bound" and "mult_bound", but it doesn't accept those keywords as valid arguments. Should we add <code>*args, **kwds</code> to the argument list? Or should those keywords be dealt with explicitly? I tried adding generic <code>*args</code> and <code>**kwds</code>, but doctesting on that file segfaults.
</p>
<p>
When I doctest the whole Sage library after making these changes, I get the following failures:
</p>
<pre class="wiki">The following tests failed:
sage -t -long -force_lib devel/sage/sage/rings/polynomial/multi_polynomial_ideal.py # Killed/crashed
sage -t -long -force_lib devel/sage/sage/rings/polynomial/multi_polynomial_libsingular.pyx # 1 doctests failed
sage -t -long -force_lib devel/sage/sage/libs/singular/polynomial.pyx # 1 doctests failed
sage -t -long -force_lib devel/sage/sage/rings/polynomial/plural.pyx # 6 doctests failed
</pre><p>
On one machine, I also had this failure:
</p>
<pre class="wiki"> sage -t -long -force_lib devel/sage/sage/rings/polynomial/multi_polynomial.pyx # Killed/crashed
</pre>
TicketjhpalmieriWed, 30 Mar 2011 22:36:18 GMT
https://trac.sagemath.org/ticket/4539#comment:42
https://trac.sagemath.org/ticket/4539#comment:42
<p>
I've rebased the patch to Sage 4.7. I'm not sure it's worth cluttering up this ticket with more patches, especially ones this big, so I've posted it to <a class="ext-link" href="http://sage.math.washington.edu/home/palmieri/misc/trac_4539_libplural-rebased.patch"><span class="icon"></span>http://sage.math.washington.edu/home/palmieri/misc/trac_4539_libplural-rebased.patch</a>. This patch also fixes a few docstrings, and it makes the changes that I described above, although I'm not sure they're the right thing to do.
</p>
TicketSimonKingMon, 26 Sep 2011 17:50:36 GMTkeywords changed
https://trac.sagemath.org/ticket/4539#comment:43
https://trac.sagemath.org/ticket/4539#comment:43
<ul>
<li><strong>keywords</strong>
<em>sd34</em> added
</li>
</ul>
<p>
At sage days 34, we try to rebase the old patch. Burcin and I agree that we should rebase it with respect to <a class="closed ticket" href="https://trac.sagemath.org/ticket/7797" title="enhancement: Full interface to letterplace from singular (closed: fixed)">#7797</a> (which, among other things, modernises coercion for free algebras). It also means that we can use one- and two-sided ideals in non-commutative rings, by <a class="closed ticket" href="https://trac.sagemath.org/ticket/11068" title="enhancement: Basic implementation of one- and twosided ideals of non-commutative ... (closed: fixed)">#11068</a>.
</p>
<p>
Some hunks of the latest patch fail due to other tickets that meanwhile are merged, such as <a class="closed ticket" href="https://trac.sagemath.org/ticket/11316" title="enhancement: Weighted degree term orders added (closed: fixed)">#11316</a>.
</p>
<p>
It seems to me that, out of the 5 hunks that fail, only 2 are non-trivial to fix.
</p>
TicketAlexanderDreyerMon, 26 Sep 2011 22:29:41 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_libplural-2011-09-27-untested.patch</em>
</li>
</ul>
<p>
experimental rebasement to 4.7.2alpha3-prerelease
</p>
TicketAlexanderDreyerMon, 26 Sep 2011 22:32:30 GMT
https://trac.sagemath.org/ticket/4539#comment:44
https://trac.sagemath.org/ticket/4539#comment:44
<p>
Can you test, whether Attach:trac4539_libplural-2011-09-27-untested.patch does the job?
</p>
TicketSimonKingMon, 26 Sep 2011 23:04:56 GMT
https://trac.sagemath.org/ticket/4539#comment:45
https://trac.sagemath.org/ticket/4539#comment:45
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:44" title="Comment 44">AlexanderDreyer</a>:
</p>
<blockquote class="citation">
<p>
Can you test, whether Attach:trac4539_libplural-2011-09-27-untested.patch does the job?
</p>
</blockquote>
<p>
We were just dubling the work, it seems. I had rebased my old patch and was running tests (don't know the outcome).
</p>
<p>
But is your patch relative to <a class="closed ticket" href="https://trac.sagemath.org/ticket/11068" title="enhancement: Basic implementation of one- and twosided ideals of non-commutative ... (closed: fixed)">#11068</a> (and perhaps to <a class="closed ticket" href="https://trac.sagemath.org/ticket/7797" title="enhancement: Full interface to letterplace from singular (closed: fixed)">#7797</a> as well)? <a class="closed ticket" href="https://trac.sagemath.org/ticket/11068" title="enhancement: Basic implementation of one- and twosided ideals of non-commutative ... (closed: fixed)">#11068</a> already has a positive review, and since it adds one- and twosided ideals of non-commutative rings, it seems like a natural dependency for a plural wrapper.
</p>
TicketAlexanderDreyerMon, 26 Sep 2011 23:15:56 GMT
https://trac.sagemath.org/ticket/4539#comment:46
https://trac.sagemath.org/ticket/4539#comment:46
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:45" title="Comment 45">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
We were just dubling the work, it seems. I had rebased my old patch and was running tests (don't know the outcome). But is your patch relative to <a class="closed ticket" href="https://trac.sagemath.org/ticket/11068" title="enhancement: Basic implementation of one- and twosided ideals of non-commutative ... (closed: fixed)">#11068</a> (and perhaps to <a class="closed ticket" href="https://trac.sagemath.org/ticket/7797" title="enhancement: Full interface to letterplace from singular (closed: fixed)">#7797</a> as well)? <a class="closed ticket" href="https://trac.sagemath.org/ticket/11068" title="enhancement: Basic implementation of one- and twosided ideals of non-commutative ... (closed: fixed)">#11068</a> already has a positive review, and since it adds one- and twosided ideals of non-commutative rings, it seems like a natural dependency for a plural wrapper.
</p>
</blockquote>
<p>
Sorry, I thought rebasing couldn't be finished. But it seems, that something is wrong with my rebased patch. So you should post yours.
</p>
TicketAlexanderDreyerMon, 26 Sep 2011 23:57:58 GMT
https://trac.sagemath.org/ticket/4539#comment:47
https://trac.sagemath.org/ticket/4539#comment:47
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:46" title="Comment 46">AlexanderDreyer</a>:
</p>
<blockquote class="citation">
<p>
Sorry, I thought rebasing couldn't be finished. But it seems, that something is wrong with my rebased patch. So you should post yours.
</p>
</blockquote>
<p>
Just foudn out, the patch was fine (so far), but my sage/devel directory was corrupted. (Luckily I cloned.)
</p>
TicketSimonKingTue, 27 Sep 2011 06:54:46 GMT
https://trac.sagemath.org/ticket/4539#comment:48
https://trac.sagemath.org/ticket/4539#comment:48
<p>
I have attached my experimental rebasement (relative to <a class="closed ticket" href="https://trac.sagemath.org/ticket/7797" title="enhancement: Full interface to letterplace from singular (closed: fixed)">#7797</a> on top of sage-4.7.2.alpha3), but not all is good.
</p>
<p>
There are quite a lot of doctest failures, including some very wrong arithmetical results in plural.pyx.
</p>
<p>
Less dramatic: There are some zero division errors (for example in doctests of mul
ti_polynomial_libsingular.pyx) where the expected error message was "rational division by zero", but now there is <em>no</em> error message (but there is a <code>ZeroDivisionError</code>, at least).
</p>
<p>
There is a segment fault in the tests of multi_polynomial_ideal.py. I guess it is the same segfault that also occurs in some elliptic curves tests.
</p>
<p>
Alexander, when you said that your broken Sage installation was to blame: Do you mean that the tests are mostly passing with your patch?
</p>
TicketSimonKingTue, 27 Sep 2011 08:12:14 GMT
https://trac.sagemath.org/ticket/4539#comment:49
https://trac.sagemath.org/ticket/4539#comment:49
<p>
I am now testing Alexander's patch. First good news: It applies without fuzz, even when <a class="closed ticket" href="https://trac.sagemath.org/ticket/7797" title="enhancement: Full interface to letterplace from singular (closed: fixed)">#7797</a> is applied. Let's see how the doc tests work!
</p>
TicketSimonKingTue, 27 Sep 2011 08:21:51 GMT
https://trac.sagemath.org/ticket/4539#comment:50
https://trac.sagemath.org/ticket/4539#comment:50
<p>
I started with "manually" testing against three of the doc test errors that I got with my patch. Two of them fail with Alexander's patch as well:
</p>
<pre class="wiki">sage: P.<x,y,z> = QQ[]
sage: x/0
Traceback (most recent call last):
...
ZeroDivisionError:
</pre><blockquote>
<p>
--> The old error message "rational division by zero" has gone.
</p>
</blockquote>
<pre class="wiki">sage: (x*y).is_monomial()
True
sage: (2*y).is_monomial()
False
</pre><blockquote>
<p>
--> That's better than my patch, where these return 1 and 0.
</p>
</blockquote>
<pre class="wiki">sage: (x+y^2^30)^10
x^10
</pre><blockquote>
<p>
--> That should result in an overflow error.
</p>
</blockquote>
<p>
I didn't analyse the segmentation faults.
</p>
TicketSimonKingTue, 27 Sep 2011 08:22:30 GMT
https://trac.sagemath.org/ticket/4539#comment:51
https://trac.sagemath.org/ticket/4539#comment:51
<p>
PS: Note that all these problems concern <em>commutative</em> polynomials.
</p>
TicketSimonKingTue, 27 Sep 2011 14:16:42 GMT
https://trac.sagemath.org/ticket/4539#comment:52
https://trac.sagemath.org/ticket/4539#comment:52
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:50" title="Comment 50">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
sage: P.<x,y,z> = QQ[]
sage: x/0
Traceback (most recent call last):
...
<a class="missing wiki">ZeroDivisionError?</a>:
</p>
</blockquote>
<p>
Here is the explanation:
</p>
<p>
Let <code>P = QQ[x,y,z]</code>. Since coercion is now done properly, x/0 is first converting 0 into P and tries to invert it there. The result is a naked <code>ZeroDivisionError</code> in
sage.libs.singular.polynomial.singular_polynomial_div_coeff.
Before, it used to invert 0 as a rational number, resulting in a <code>ZeroDivisionError</code> with some error message.
</p>
<p>
Burcin and I agree that it is ok to have the <code>ZeroDivisionError</code> without a message: What else could it state but "don't divide by zero"?
</p>
<p>
So, it is not an issue.
</p>
<blockquote class="citation">
<pre class="wiki">sage: (x*y).is_monomial()
True
sage: (2*y).is_monomial()
False
</pre></blockquote>
<p>
I don't know why it has occured in the first place, but now it seems alright, even with my patch.
</p>
<blockquote class="citation">
<pre class="wiki">sage: (x+y^2^30)^10
x^10
</pre><blockquote>
<p>
--> That should result in an overflow error.
</p>
</blockquote>
</blockquote>
<p>
It turns out that one gets the <em>same</em> stupid result with an unpatched sage-4.7.2.alpha2. Burcin told me that this patch is supposed to fix it. Apparently it fails, and we need to understand why it fails.
</p>
<blockquote class="citation">
<p>
I didn't analyse the segmentation faults.
</p>
</blockquote>
<p>
That will be next...
</p>
TicketSimonKingTue, 27 Sep 2011 14:38:29 GMT
https://trac.sagemath.org/ticket/4539#comment:53
https://trac.sagemath.org/ticket/4539#comment:53
<p>
I created a new ticket <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a> for the (missing) overflow error.
</p>
TicketSimonKingTue, 27 Sep 2011 14:39:03 GMTdependencies set
https://trac.sagemath.org/ticket/4539#comment:54
https://trac.sagemath.org/ticket/4539#comment:54
<ul>
<li><strong>dependencies</strong>
set to <em>#11856</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:53" title="Comment 53">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I created a new ticket <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a> for the (missing) overflow error.
</p>
</blockquote>
<p>
Sorry, I forgot: It should be a dependency.
</p>
TicketSimonKingTue, 27 Sep 2011 16:10:14 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_libplural_todo.patch</em>
</li>
</ul>
<p>
Experimental rebasement wrt sage-4.7.2.alpha3 plus <a class="closed ticket" href="https://trac.sagemath.org/ticket/7797" title="enhancement: Full interface to letterplace from singular (closed: fixed)">#7797</a> plus <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a>
</p>
TicketSimonKingTue, 27 Sep 2011 16:13:37 GMTdependencies changed
https://trac.sagemath.org/ticket/4539#comment:55
https://trac.sagemath.org/ticket/4539#comment:55
<ul>
<li><strong>dependencies</strong>
changed from <em>#11856</em> to <em>#7797 #11316 #11856</em>
</li>
</ul>
<p>
It seems that I was able to get the missing overflow error in <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a>, which I added as a dependency. I had to update my patch (and Alexander will need to update his as well), because the function overflow_check is now expecting two arguments (a long and a ring), not one.
</p>
<p>
Tests are still missing, of course, and I have still no idea about the segfault.
</p>
TicketAlexanderDreyerTue, 27 Sep 2011 22:07:40 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_lmul.patch</em>
</li>
</ul>
<p>
fixes at least some segfault (updated patch) - needs main patch applied before
</p>
TicketAlexanderDreyerTue, 27 Sep 2011 22:11:45 GMT
https://trac.sagemath.org/ticket/4539#comment:56
https://trac.sagemath.org/ticket/4539#comment:56
<p>
I found out that the intended lmul implementation, namely using rmul <em>and</em> reverting left and right hand side, is an illegal for some right hand side objects. Up to now, this is only verified for schemes/elliptic_curves/ell_curve_isogeny.py More extensive tests follow.
</p>
TicketSimonKingWed, 28 Sep 2011 06:16:41 GMT
https://trac.sagemath.org/ticket/4539#comment:57
https://trac.sagemath.org/ticket/4539#comment:57
<p>
Hi Alexander, the description of your lmul patch says "needs main patch applied before". Which of the two main patches are you referring to?
</p>
TicketAlexanderDreyerWed, 28 Sep 2011 08:59:07 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_kwds.patch</em>
</li>
</ul>
<p>
fixes "keyword not found" issue (revert unnecessary path of the big patch)
</p>
TicketAlexanderDreyerWed, 28 Sep 2011 09:01:19 GMT
https://trac.sagemath.org/ticket/4539#comment:58
https://trac.sagemath.org/ticket/4539#comment:58
<p>
Here another small patch reverts an unnecessary part of the big patch. It fixes the keyword argument not found issue.
</p>
TicketAlexanderDreyerWed, 28 Sep 2011 10:19:38 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_is_monomial.patch</em>
</li>
</ul>
TicketAlexanderDreyerWed, 28 Sep 2011 10:33:49 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_overflow.patch</em>
</li>
</ul>
<p>
Patch for using trac4539_libplural-2011-09-27-untested.patch together with <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a> (not needed for trac4539_libplural_todo.patch)
</p>
TicketAlexanderDreyerWed, 28 Sep 2011 10:54:41 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_monomial_quotient.patch</em>
</li>
</ul>
<p>
monomial_quotient should throw instead of return nonsense
</p>
TicketSimonKingWed, 28 Sep 2011 13:18:13 GMT
https://trac.sagemath.org/ticket/4539#comment:59
https://trac.sagemath.org/ticket/4539#comment:59
<p>
Concerning <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/trac4539_monomial_quotient.patch" title="Attachment 'trac4539_monomial_quotient.patch' in Ticket #4539">trac4539_monomial_quotient.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/trac4539_monomial_quotient.patch" title="Download"></a>: I am not sure if it is the right thing to do. I think that monomial_quotient is a method that should be as fast as possible, since in some situations it is used very frequently. In these situations, it is always the case that one monomial <em>does</em> divide the other. Hence, for the application, it is a bad idea to have a redundant sanity test in monomial_quotient. I'd rather have it return a wrong result when using it in a wrong way.
</p>
<p>
Note that <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/trac4539_kwds.patch" title="Attachment 'trac4539_kwds.patch' in Ticket #4539">trac4539_kwds.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/trac4539_kwds.patch" title="Download"></a> is not needed for my patch - I already have *args in it.
</p>
<p>
We have already briefly discussed why I think that <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/trac4539_lmul.patch" title="Attachment 'trac4539_lmul.patch' in Ticket #4539">trac4539_lmul.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/trac4539_lmul.patch" title="Download"></a> probably is not a good approach: x._rmul_(c) and x._lmul_(c) (by specification of the coercion model) can assume that the argument c belongs to x.parent().base_ring(). In particular, I don't believe that c can actually be a non-commutative polynomial.
</p>
<p>
Can you please provide an example that was segfaulting without the lmul-patch?
</p>
TicketSimonKingWed, 28 Sep 2011 13:38:19 GMT
https://trac.sagemath.org/ticket/4539#comment:60
https://trac.sagemath.org/ticket/4539#comment:60
<p>
At least, when I use <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/trac4539_libplural_todo.patch" title="Attachment 'trac4539_libplural_todo.patch' in Ticket #4539">trac4539_libplural_todo.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/trac4539_libplural_todo.patch" title="Download"></a> plus <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/trac4539_lmul.patch" title="Attachment 'trac4539_lmul.patch' in Ticket #4539">trac4539_lmul.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/trac4539_lmul.patch" title="Download"></a> on top of sage-4.7.3.alpha3-prerelease, then all tests in sage/rings/polynomial pass (except those that fail in the unpatched version as well).
</p>
<p>
Since I believe that the monomial quotient patch does not do the right thing, I'd prefer to work on top of <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/trac4539_libplural_todo.patch" title="Attachment 'trac4539_libplural_todo.patch' in Ticket #4539">trac4539_libplural_todo.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/trac4539_libplural_todo.patch" title="Download"></a> and <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/trac4539_lmul.patch" title="Attachment 'trac4539_lmul.patch' in Ticket #4539">trac4539_lmul.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/trac4539_lmul.patch" title="Download"></a>, and concentrate on getting the lmul business right.
</p>
TicketSimonKingWed, 28 Sep 2011 13:40:21 GMT
https://trac.sagemath.org/ticket/4539#comment:61
https://trac.sagemath.org/ticket/4539#comment:61
<p>
I can already confirm that ther <em>is</em> a segfault without the lmul patch. So, I'll try to analyse what arguments are passed to _rmul_ / _lmul_. If I remember correctly, polynomial rings do not use the current coercion model. Hence, it is perhaps no surprise that _rmul_ and _lmul_ do not do what we expect.
</p>
TicketSimonKingWed, 28 Sep 2011 13:51:50 GMT
https://trac.sagemath.org/ticket/4539#comment:62
https://trac.sagemath.org/ticket/4539#comment:62
<p>
It is very strange: Apparently, a different lmul method fixes the segfault. However, when I insert a print statement in the unpatched lmul method, then I find that it is actually not executed directly before segfaulting.
</p>
<p>
Alexander, how did you found that a change in lmul could fix it?
</p>
TicketSimonKingWed, 28 Sep 2011 14:07:50 GMT
https://trac.sagemath.org/ticket/4539#comment:63
https://trac.sagemath.org/ticket/4539#comment:63
<p>
Alexander has just explained the lmul problem to me. It was an oversight in the original patch, where self._lmul_(right) was calling right._rmul_(self), which is of course wrong, since the argument to _rmul_ must be an element of the base ring. It should correctly be self._rmul_(right).
</p>
<p>
Alexander just told me that he agrees in dropping the (redundant) test in monomial_quotient.
</p>
TicketSimonKingWed, 28 Sep 2011 14:10:45 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_libplural.2.patch</em>
</li>
</ul>
<p>
Combined patch relative to sage-4.7.2.alpha3 plus <a class="closed ticket" href="https://trac.sagemath.org/ticket/7797" title="enhancement: Full interface to letterplace from singular (closed: fixed)">#7797</a> plus <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a>
</p>
TicketSimonKingWed, 28 Sep 2011 14:14:47 GMTstatus, description changed
https://trac.sagemath.org/ticket/4539#comment:64
https://trac.sagemath.org/ticket/4539#comment:64
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/4539?action=diff&version=64">diff</a>)
</li>
</ul>
<p>
The new <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/trac4539_libplural.2.patch" title="Attachment 'trac4539_libplural.2.patch' in Ticket #4539">trac4539_libplural.2.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/trac4539_libplural.2.patch" title="Download"></a> is stand-alone and is supposed to summarise the discussion we had here. I think it is ready to be reviewed (but as usual I didn't run the tests yet...).
</p>
<p>
Apply trac4539_libplural.2.patch
</p>
TicketSimonKingWed, 28 Sep 2011 15:33:47 GMTstatus changed; work_issues set
https://trac.sagemath.org/ticket/4539#comment:65
https://trac.sagemath.org/ticket/4539#comment:65
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>work_issues</strong>
set to <em>detect the doctest with a verbosity side effect</em>
</li>
</ul>
<p>
I get a doctest failure in sage/rings/polynomial/multi_polynomial_ideal.py. There, a protocol of a Gröbner basis computation is printed where we do not expect it.
</p>
<p>
The problem: If one runs the test separately, it works fine:
</p>
<pre class="wiki">sage: A.<x,y,z> = FreeAlgebra(QQ, 3)
sage: H = A.g_algebra({y*x:x*y-z, z*x:x*z+2*x, z*y:y*z-2*y})
sage: H.inject_variables()
Defining x, y, z
sage: I = H.ideal([y^2, x^2, z^2-H.one_element()],coerce=False)
sage: G = vector(I.gens()); G
/mnt/local/king/SAGE/debug/sage-4.7.2.alpha3-prerelease/local/lib/python2.6/site-packages/sage/modules/free_module.py:366: UserWarning: You are constructing a free module over a noncommutative ring. Sage does
not have a concept of left/right and both sided modules, so be careful. It's also
not guaranteed that all multiplications are done from the right side.
not guaranteed that all multiplications are done from the right side.""")
/mnt/local/king/SAGE/debug/sage-4.7.2.alpha3-prerelease/local/lib/python2.6/site-packages/sage/modules/free_module.py:584: UserWarning: You are constructing a free module over a noncommutative ring. Sage does not have a concept of left/right and both sided modules be careful. It's also not guarantied that all multiplications are done from the right side.
warn("""You are constructing a free module over a noncommutative ring. Sage does not have a concept of left/right and both sided modules be careful. It's also not guarantied that all multiplications are done from the right side.""")
(y^2, x^2, z^2 - 1)
</pre><p>
But the same doctest executed as part of the doctest suite has a
</p>
<pre class="wiki">std in (0),(x,y),(dp(2),C)
[4294967295:2]3ss4s6
(S:2)--
product criterion:1 chain criterion:0
</pre><p>
printed after (!!) the result.
</p>
<p>
So, apparently another test has a side effect. I tried to identify it (e.g., a test that sets verbosity and forgets to reset it), but I did not succeed. Also I wonder why one first sees the result and only later sees the protocol.
</p>
TicketSimonKingWed, 28 Sep 2011 17:09:21 GMTstatus, keywords, description changed
https://trac.sagemath.org/ticket/4539#comment:66
https://trac.sagemath.org/ticket/4539#comment:66
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>keywords</strong>
<em>sd10</em> <em>sd23.5</em> <em>sd24</em> added
</li>
<li><strong>description</strong>
modified (<a href="/ticket/4539?action=diff&version=66">diff</a>)
</li>
</ul>
<p>
It turned out that I misinterpreted the error: The actual error was wrong line breaks in the expected warning message. The protocol from Singular is printed to stdout, and apparently it was just by accident (though reproducible) that I saw it in the test log.
</p>
<p>
Note that the warning message misspells the word "guaranteed" (namely "guarantied"). I fixed that misspelling as well, and I also introduced nicer (I think) line breaks for the warning.
</p>
<p>
Glad that this is fixed. I hope the tests pass by now.
</p>
<p>
Apply trac4539_libplural.patch
</p>
TicketSimonKingWed, 28 Sep 2011 17:48:58 GMT
https://trac.sagemath.org/ticket/4539#comment:67
https://trac.sagemath.org/ticket/4539#comment:67
<p>
FWIW, all doctests pass for me, except those that fail with unpatched sage-4.7.2.alpha3-prerelease.
</p>
<p>
By the way: How should reviewing be done in this case? I have no overview who wrote what (i.e., who can <em>review</em> what), and I think many people contributed to it.
</p>
<p>
In <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/trac4539_libplural.patch" title="Attachment 'trac4539_libplural.patch' in Ticket #4539">trac4539_libplural.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/trac4539_libplural.patch" title="Download"></a>, I added an author list. But is it exhaustive?
</p>
TicketSimonKingWed, 28 Sep 2011 18:19:51 GMT
https://trac.sagemath.org/ticket/4539#comment:68
https://trac.sagemath.org/ticket/4539#comment:68
<p>
Concerning reviewing: Would it be OK that we all comment whether we are happy with the current patch, and that it constitutes a positive review if all are happy with it and nobody has a veto? Martin seems to agree.
</p>
TicketSimonKingWed, 28 Sep 2011 18:33:59 GMT
https://trac.sagemath.org/ticket/4539#comment:69
https://trac.sagemath.org/ticket/4539#comment:69
<p>
I am writing a reviewer patch, since several doc strings needs reformatting.
</p>
<p>
Question:
</p>
<p>
In sage/algebras/free_algebra.py in the method g_algebra, I find the statement:
"By default is assumed, that two variables commute." I don't understand that statement. Is it meant "If there are only two variables then they commute"? Or "Any two variables commute" (hopefully not)? Or "There are two variables that commute" (but which)? Can you provide an example for that default, and also show how (if possible) the default can be overridden?
</p>
TicketSimonKingWed, 28 Sep 2011 18:39:16 GMTstatus changed
https://trac.sagemath.org/ticket/4539#comment:70
https://trac.sagemath.org/ticket/4539#comment:70
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_info</em>
</li>
</ul>
<p>
PS: The other statements
</p>
<pre class="wiki"> - Coercion doesn't work yet, there is some cheating about assumptions
- The optional argument ``check`` controls checking the degeneracy conditions.
Furthermore, the default values interfere with non-degeneracy conditions.
</pre><p>
aren't clear to me either.
</p>
<ul><li>What does "cheating about assumptions" mean (what are the assumptions)?
</li><li>What exactly does not work in coercion (perhaps I can fix it?)?
</li><li>What are "the default values" (the only default is <code>order="degrevlex"</code>)?
</li><li>How do they interfere with non-degeneracy conditions? What are these conditions?
</li></ul>
TicketSimonKingWed, 28 Sep 2011 18:57:44 GMT
https://trac.sagemath.org/ticket/4539#comment:71
https://trac.sagemath.org/ticket/4539#comment:71
<p>
Too bad. I forgot to apply my "combined" patch relative to <a class="closed ticket" href="https://trac.sagemath.org/ticket/7797" title="enhancement: Full interface to letterplace from singular (closed: fixed)">#7797</a> (or at least to <a class="closed ticket" href="https://trac.sagemath.org/ticket/11068" title="enhancement: Basic implementation of one- and twosided ideals of non-commutative ... (closed: fixed)">#11068</a>). Needs work.
</p>
TicketSimonKingWed, 28 Sep 2011 19:03:17 GMTstatus, dependencies changed
https://trac.sagemath.org/ticket/4539#comment:72
https://trac.sagemath.org/ticket/4539#comment:72
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
<li><strong>dependencies</strong>
changed from <em>#7797 #11316 #11856</em> to <em>#11068 #11316 #11856</em>
</li>
</ul>
<p>
I think that it would be better to just have <a class="closed ticket" href="https://trac.sagemath.org/ticket/11068" title="enhancement: Basic implementation of one- and twosided ideals of non-commutative ... (closed: fixed)">#11068</a> as a dependency: We should use it (because it implements one- and twosided ideals), and in contrast to <a class="closed ticket" href="https://trac.sagemath.org/ticket/7797" title="enhancement: Full interface to letterplace from singular (closed: fixed)">#7797</a> it has a positive review.
</p>
TicketSimonKingWed, 28 Sep 2011 19:03:52 GMTstatus, work_issues changed
https://trac.sagemath.org/ticket/4539#comment:73
https://trac.sagemath.org/ticket/4539#comment:73
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>work_issues</strong>
changed from <em>detect the doctest with a verbosity side effect</em> to <em>rebase wrt #11068; doc string formatting</em>
</li>
</ul>
TicketSimonKingThu, 29 Sep 2011 00:06:58 GMTwork_issues changed
https://trac.sagemath.org/ticket/4539#comment:74
https://trac.sagemath.org/ticket/4539#comment:74
<ul>
<li><strong>work_issues</strong>
changed from <em>rebase wrt #11068; doc string formatting</em> to <em>Pickling. Quotients. Uniqueness.</em>
</li>
</ul>
<p>
Here's a report on features that I implemented today (not posted yet), missing features, and I also have some questions for you:
</p>
<p>
<span class="underline">Sidedness of ideals</span>
</p>
<p>
By <a class="closed ticket" href="https://trac.sagemath.org/ticket/11068" title="enhancement: Basic implementation of one- and twosided ideals of non-commutative ... (closed: fixed)">#11068</a>, we can have one- and two-sided ideals. Oleksander told me that Singular can only compute left or twosided Gröbner bases. Therefore, I think non-commutative polynomial ideals should refuse to be right-sided. The default should be left ideal. If the ideal is defined as a two-sided ideal, then std should return the same as twostd. Here is an example:
</p>
<pre class="wiki">sage: A.<x,y,z> = FreeAlgebra(QQ, 3)
sage: H = A.g_algebra({y*x:x*y-z, z*x:x*z+2*x, z*y:y*z-2*y})
sage: H.inject_variables()
Defining x, y, z
sage: JL = H.ideal([x^3, y^3, z^3 - 4*z])
sage: JL
Left Ideal (x^3, y^3, z^3 - 4*z) of Noncommutative Multivariate Polynomial Ring in x, y, z over Rational Field, nc-relations: {y*x: x*y - z, z*y: y*z - 2*y, z*x: x*z + 2*x}
sage: JL.std()
Left Ideal (z^3 - 4*z, y*z^2 - 2*y*z, x*z^2 + 2*x*z, 2*x*y*z - z^2 - 2*z, y^3, x^3) of Noncommutative Multivariate Polynomial Ring in x, y, z over Rational Field, nc-relations: {y*x: x*y - z, z*y: y*z - 2*y, z*x: x*z + 2*x}
sage: JT = H.ideal([x^3, y^3, z^3 - 4*z], side='twosided')
sage: JT
Twosided Ideal (x^3, y^3, z^3 - 4*z) of Noncommutative Multivariate Polynomial Ring in x, y, z over Rational Field, nc-relations: {y*x: x*y - z, z*y: y*z - 2*y, z*x: x*z + 2*x}
sage: JT.std()
Twosided Ideal (z^3 - 4*z, y*z^2 - 2*y*z, x*z^2 + 2*x*z, y^2*z - 2*y^2, 2*x*y*z - z^2 - 2*z, x^2*z + 2*x^2, y^3, x*y^2 - y*z, x^2*y - x*z - 2*x, x^3) of Noncommutative Multivariate Polynomial Ring in x, y, z over Rational Field, nc-relations: {y*x: x*y - z, z*y: y*z - 2*y, z*x: x*z + 2*x}
sage: JT.std() == JL.twostd()
True
</pre><p>
I think that's a good solution.
</p>
<p>
<span class="underline">Cache</span>
</p>
<p>
I added a cached_method decorator to std and twostd - I guess it is obvious that the result of a GB computation should be cached.
</p>
<p>
<strong>Question:</strong> Should g-algebras be unique parents? If you agree that they should be, then I can try to implement it.
</p>
<p>
<span class="underline">Category</span>
</p>
<p>
A proper initialisation of non-commutative polynomial rings in the category of algebras was missing and is now added:
</p>
<pre class="wiki">sage: H._is_category_initialized()
True
sage: H.category()
Category of algebras over Rational Field
</pre><p>
<span class="underline">Pickling</span>
</p>
<p>
The test suite does not pass. Among other things, pickling of g-algebras has simply been forgotten. This certainly must be fixed:
</p>
<pre class="wiki">sage: dumps(H)
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
...
TypeError: expected string or Unicode object, NoneType found
</pre><p>
It could be that, by using a <code>UniqueFactory</code> or <code>UniqueRepresentation</code>, the pickling problem automatically vanishes. Otherwise, a <code>__reduce__</code> method must be implemented.
</p>
<p>
<span class="underline">Generator names for g-algebras</span>
</p>
<p>
It should be possible to choose names in the <code>g_algebra()</code> method.
</p>
<p>
<span class="underline">Quotients</span>
</p>
<p>
There is a custom <code>quotient()</code> method for g-algebras (not using <a class="closed ticket" href="https://trac.sagemath.org/ticket/11068" title="enhancement: Basic implementation of one- and twosided ideals of non-commutative ... (closed: fixed)">#11068</a>). The question is: Is that really a quotient? It isn't printed as such, and the quotient relations are not used in arithmetic nor in comparison:
</p>
<pre class="wiki">sage: A.<x,y,z> = FreeAlgebra(QQ, 3)
sage: H = A.g_algebra({y*x:x*y-z, z*x:x*z+2*x, z*y:y*z-2*y})
sage: H.inject_variables()
Defining x, y, z
sage: I = H.ideal([y^2, x^2, z^2-H.one_element()],coerce=False)
sage: Q = H.quotient(I); Q
Noncommutative Multivariate Polynomial Ring in x, y, z over Rational Field, nc-relations: {y*x: x*y - z, z*y: y*z - 2*y, z*x: x*z + 2*x}
sage: Q.relations()
{y*x: x*y - z, z*y: y*z - 2*y, z*x: x*z + 2*x}
sage: I.twostd()
Twosided Ideal (z^2 - 1, y*z - y, x*z + x, y^2, 2*x*y - z - 1, x^2) of Noncommutative Multivariate Polynomial Ring in x, y, z over Rational Field, nc-relations: {y*x: x*y - z, z*y: y*z - 2*y, z*x: x*z + 2*x}
sage: Q.2^2
z^2
sage: Q.2^2 == Q.one_element()
False
</pre><p>
<strong>Question:</strong> Did I misinterprete it? Hence, is there a way to make Q show that <code>Q.2^2</code> is equal to one?
</p>
<p>
Otherwise, the custom <code>quotient()</code> should be dropped. <a class="closed ticket" href="https://trac.sagemath.org/ticket/11068" title="enhancement: Basic implementation of one- and twosided ideals of non-commutative ... (closed: fixed)">#11068</a> provides the framework for nc-quotient rings; one just needs to add a <code>I.reduce(p)</code> method to our ideals (which is missing anyway).
</p>
<p>
<span class="underline">Doc strings</span>
</p>
<p>
I fixed various wrong doc string formats. Of course, after the changes mentioned above, some doc tests need to be modified.
</p>
TicketSimonKingThu, 29 Sep 2011 08:10:33 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_libplural.patch</em>
</li>
</ul>
<p>
Combined patch relative to sage-4.7.2.alpha3 plus <a class="closed ticket" href="https://trac.sagemath.org/ticket/11068" title="enhancement: Basic implementation of one- and twosided ideals of non-commutative ... (closed: fixed)">#11068</a> plus <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a>
</p>
TicketSimonKingThu, 29 Sep 2011 08:11:46 GMT
https://trac.sagemath.org/ticket/4539#comment:75
https://trac.sagemath.org/ticket/4539#comment:75
<p>
New patch posted! It does what I have announced above. I suggest that the next step should be to provide pickling, possibly by using <code>UniqueRepresentation</code>.
</p>
TicketSimonKingThu, 29 Sep 2011 10:38:07 GMTdescription changed
https://trac.sagemath.org/ticket/4539#comment:76
https://trac.sagemath.org/ticket/4539#comment:76
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4539?action=diff&version=76">diff</a>)
</li>
</ul>
<p>
The patch that I have just attached provides uniqueness of the parent (using a <code>UniqueFactory</code>) and pickling for nc rings and polynomials.
</p>
<p>
In short:
</p>
<pre class="wiki">sage: A.<x,y,z> = FreeAlgebra(QQ, 3)
sage: H = A.g_algebra({y*x:x*y-z, z*x:x*z+2*x, z*y:y*z-2*y})
sage: H is loads(dumps(H))
True
sage: TestSuite(H).run()
</pre><p>
Doc tests still need to be updated, though. I also think that normal form computation should be easy to implement.
</p>
<p>
Apply trac4539_libplural.patch trac4539_pickling.patch
</p>
TicketSimonKingThu, 29 Sep 2011 11:31:06 GMTwork_issues changed
https://trac.sagemath.org/ticket/4539#comment:77
https://trac.sagemath.org/ticket/4539#comment:77
<ul>
<li><strong>work_issues</strong>
changed from <em>Pickling. Quotients. Uniqueness.</em> to <em>Quotients and normal forms</em>
</li>
</ul>
TicketSimonKingThu, 29 Sep 2011 12:28:22 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_pickling.patch</em>
</li>
</ul>
<p>
Pickling of nc rings and polynomials
</p>
TicketSimonKingThu, 29 Sep 2011 12:35:07 GMTdescription changed
https://trac.sagemath.org/ticket/4539#comment:78
https://trac.sagemath.org/ticket/4539#comment:78
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/4539?action=diff&version=78">diff</a>)
</li>
</ul>
<p>
I have updated the second patch (adding a commit message), and I added a third patch. It provides a non-commutative "Gröbner strategy", normal form computation, and thus quotient rings of g-algebras.
</p>
<p>
Note that the quotients use the general framework from <a class="closed ticket" href="https://trac.sagemath.org/ticket/11068" title="enhancement: Basic implementation of one- and twosided ideals of non-commutative ... (closed: fixed)">#11068</a>. They <em>should</em> simply be g-algebras as well. But I suggest that this will be done on a different ticket.
</p>
<p>
With the new patch, one can do:
</p>
<pre class="wiki">sage: A.<x,y,z> = FreeAlgebra(QQ, 3)
sage: H.<x,y,z> = A.g_algebra({y*x:x*y-z, z*x:x*z+2*x, z*y:y*z-2*y})
sage: I = H.ideal([y^2, x^2, z^2-H.one_element()],coerce=False, side='twosided')
sage: Q = H.quotient(I); Q
Quotient of Noncommutative Multivariate Polynomial Ring in x, y, z
over Rational Field, nc-relations: {y*x: x*y - z, z*y: y*z - 2*y,
z*x: x*z + 2*x} by the ideal (y^2, x^2, z^2 - 1)
sage: Q.2^2 == Q.one_element() # indirect doctest
True
</pre><p>
Here, we see that the relation that we just found in the quotient is actually a consequence of the given relations:
</p>
<pre class="wiki">sage: I.twostd()
Twosided Ideal (z^2 - 1, y*z - y, x*z + x, y^2, 2*x*y - z - 1, x^2)
of Noncommutative Multivariate Polynomial Ring in x, y, z over
Rational Field, nc-relations: {y*x: x*y - z, z*y: y*z - 2*y, z*x: x*z + 2*x}
</pre><p>
Note that reduction of polynomials by a list of polynomials is, in general, not a normal form. However, reduction of a polynomial by an ideal uses a two-sided Gröbner basis and is thus a normal form.
</p>
<p>
I just thought that it would better be reduction by a left Gröbner basis, if the ideal is just a left ideal. OK, doing it soon...
</p>
<p>
Apply trac4539_libplural.patch trac4539_pickling.patch trac4539_normal_forms.patch
</p>
TicketSimonKingThu, 29 Sep 2011 12:39:43 GMTwork_issues changed
https://trac.sagemath.org/ticket/4539#comment:79
https://trac.sagemath.org/ticket/4539#comment:79
<ul>
<li><strong>work_issues</strong>
changed from <em>Quotients and normal forms</em> to <em>doc tests</em>
</li>
</ul>
<p>
The third patch is now modified, so that reduction wrt a left ideal is computed with a left (not a two-sided) Gröbner basis.
</p>
<p>
It needs work, since the documentation is not complete and since certainly several doc tests need to be modified. But feel free to test the new patches...
</p>
TicketSimonKingThu, 29 Sep 2011 12:53:08 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_normal_forms.patch</em>
</li>
</ul>
<p>
Normal forms, quotient rings, and ideal containment
</p>
TicketSimonKingThu, 29 Sep 2011 12:56:09 GMT
https://trac.sagemath.org/ticket/4539#comment:80
https://trac.sagemath.org/ticket/4539#comment:80
<p>
Sorry, I couldn't resist to add one more feature: Ideal containment, which is a direct application of normal form computation.
</p>
<p>
With the new version of the third patch, we have:
</p>
<pre class="wiki">sage: A.<x,y,z> = FreeAlgebra(QQ, 3)
sage: H.<x,y,z> = A.g_algebra({y*x:x*y-z, z*x:x*z+2*x, z*y:y*z-2*y})
sage: JL = H.ideal([x^3, y^3, z^3 - 4*z])
sage: JL.std()
Left Ideal (z^3 - 4*z, y*z^2 - 2*y*z, x*z^2 + 2*x*z, 2*x*y*z - z^2 - 2*z, y^3, x^3) of Noncommutative Multivariate Polynomial Ring in x, y, z over Rational Field, nc-relations: {y*x: x*y - z, z*y: y*z - 2*y, z*x: x*z + 2*x}
sage: JT = H.ideal([x^3, y^3, z^3 - 4*z], side='twosided')
sage: JT.std()
Twosided Ideal (z^3 - 4*z, y*z^2 - 2*y*z, x*z^2 + 2*x*z, y^2*z - 2*y^2, 2*x*y*z - z^2 - 2*z, x^2*z + 2*x^2, y^3, x*y^2 - y*z, x^2*y - x*z - 2*x, x^3) of Noncommutative Multivariate Polynomial Ring in x, y, z over Rational Field, nc-relations: {y*x: x*y - z, z*y: y*z - 2*y, z*x: x*z + 2*x}
</pre><p>
Apparently, <code></code>x*y<sup>2-y*z<code></code> should be in the two-sided, but not in the left ideal. And that is indeed what we get:
</sup></p>
<pre class="wiki">sage: x*y^2-y*z in JL
False
sage: x*y^2-y*z in JT
True
</pre><p>
Docs are still to fix. And I promise to focus on it - no new features...
</p>
<p>
Apply trac4539_libplural.patch trac4539_pickling.patch trac4539_normal_forms.patch
</p>
TicketSimonKingThu, 29 Sep 2011 15:19:00 GMTstatus, description changed; reviewer, author set; work_issues deleted
https://trac.sagemath.org/ticket/4539#comment:81
https://trac.sagemath.org/ticket/4539#comment:81
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Simon King</em>
</li>
<li><strong>author</strong>
set to <em>Michael Brickenstein, Burcin Erocal, Oleksandr Motsak, Alexander Dreyer, Simon King</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/4539?action=diff&version=81">diff</a>)
</li>
<li><strong>work_issues</strong>
<em>doc tests</em> deleted
</li>
</ul>
<p>
The fourth patch does not provide a new feature, but only fixes of bugs, of the doc string formatting, and of the doc tests.
</p>
<p>
Since I do not have Sage locally, I'd appreciate if one of you could build the documentation and see if it looks nice.
</p>
<p>
Not a feature, but a fix concerns the hash: If one constructs a g-algebra as one is supposed to (<code>g_algeba</code> method resp. unique factory) then the g-algebra is a unique parent. Hence, <code>id(self)</code> is a good hash for it, and <code>__cmp__</code> can be removed.
</p>
<p>
Note that the hash in Sage is allowed to change from session to session, so, <code>id(self)</code> is fine - see <code>UniqueRepresentation.__hash__</code>. Of course, one can destroy the uniqueness on purpose, but that's not our problem.
</p>
<p>
Tests in sage/libs/singular, multi_polynomial_ideal.py and plural.pyx pass. I am now running all doc tests, but I think it is OK to put it as "needs review".
</p>
<p>
Concerning review: I think we should review each other's code. So, in particular, one of you should please review my last three patches.
</p>
<p>
Please verify if I got the credits (author list) right.
</p>
<p>
Apply trac4539_libplural.patch trac4539_pickling.patch trac4539_normal_forms.patch trac4539_fix_docs.patch
</p>
TicketSimonKingThu, 29 Sep 2011 15:45:02 GMTstatus changed; work_issues set
https://trac.sagemath.org/ticket/4539#comment:82
https://trac.sagemath.org/ticket/4539#comment:82
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>work_issues</strong>
set to <em>fix remaining doctest errors</em>
</li>
</ul>
<p>
Hoorray, only three errors, in sage/rings/noncommutative_ideal.pyx! That should be doable before dinner.
</p>
TicketSimonKingThu, 29 Sep 2011 16:29:25 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_fix_docs.patch</em>
</li>
</ul>
<p>
Fixing doc strings and doc tests
</p>
TicketSimonKingThu, 29 Sep 2011 16:32:53 GMTstatus changed
https://trac.sagemath.org/ticket/4539#comment:83
https://trac.sagemath.org/ticket/4539#comment:83
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
It turned out that the element constructor of ideal monoids changed a left ideal into a twosided ideal, which (together with the missing uniqueness of ideal monoids) led to errors in a <code>loads(dumps(...)==...</code> test.
</p>
<p>
I hope all tests will pass by now!
</p>
<p>
Apply trac4539_libplural.patch trac4539_pickling.patch trac4539_normal_forms.patch trac4539_fix_docs.patch
</p>
TicketSimonKingThu, 29 Sep 2011 20:52:27 GMTwork_issues deleted
https://trac.sagemath.org/ticket/4539#comment:84
https://trac.sagemath.org/ticket/4539#comment:84
<ul>
<li><strong>work_issues</strong>
<em>fix remaining doctest errors</em> deleted
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:83" title="Comment 83">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I hope all tests will pass by now!
</p>
</blockquote>
<p>
They do!
</p>
TicketAlexanderDreyerThu, 29 Sep 2011 21:38:00 GMT
https://trac.sagemath.org/ticket/4539#comment:85
https://trac.sagemath.org/ticket/4539#comment:85
<p>
I checked the patches, the look good indeed. So a positive review for the mathematical part. I'm starting doc tests now.
</p>
TicketAlexanderDreyerThu, 29 Sep 2011 22:10:16 GMT
https://trac.sagemath.org/ticket/4539#comment:86
https://trac.sagemath.org/ticket/4539#comment:86
<p>
Hm, trac4539_fix_docs.patch doesn't apply cleanly, maybe i used the wrong order. What does the following tell you?
<code>hg qseries</code>
</p>
TicketSimonKingFri, 30 Sep 2011 06:16:13 GMT
https://trac.sagemath.org/ticket/4539#comment:87
https://trac.sagemath.org/ticket/4539#comment:87
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:86" title="Comment 86">AlexanderDreyer</a>:
</p>
<blockquote class="citation">
<p>
Hm, trac4539_fix_docs.patch doesn't apply cleanly, maybe i used the wrong order. What does the following tell you?
</p>
</blockquote>
<pre class="wiki">hg qseries
trac11815_format_must_preserve_embedding.patch
trac11817_question_mark_using_sage_getdoc.patch
trac11768_source_of_dynamic_class.patch
trac11115-cached_cython.patch
trac11115_element_with_cache.patch
trac11115_cached_function_pickling.patch
trac11791_dynamic_metaclass_introspection.patch
trac11780_unique_auxiliar_polyring.patch
trac11856_exponent_overflow.patch
trac11068_nc_ideals_and_quotients.patch
trac11068_quotient_ring_without_names.patch
trac11068_lifting_map.patch
trac4539_libplural.patch
trac4539_pickling.patch
trac4539_normal_forms.patch
trac4539_fix_docs.patch
</pre><p>
So, as you can see, I indeed have more stuff applied in front of the plural patches. I will try to see what went wrong.
</p>
TicketSimonKingFri, 30 Sep 2011 08:13:26 GMT
https://trac.sagemath.org/ticket/4539#comment:88
https://trac.sagemath.org/ticket/4539#comment:88
<p>
I can not confirm Alexander's statement that some patch does not apply.
</p>
<p>
I cleaned my patch queue, so that I only use the patches that are dependencies. Now, I have
</p>
<pre class="wiki">$ hg qapplied
trac11815_format_must_preserve_embedding.patch
trac11115-cached_cython.patch
trac11115_element_with_cache.patch
trac11115_cached_function_pickling.patch
trac11068_nc_ideals_and_quotients.patch
trac11068_quotient_ring_without_names.patch
trac11068_lifting_map.patch
trac11856_exponent_overflow.patch
trac4539_libplural.patch
trac4539_pickling.patch
trac4539_normal_forms.patch
trac4539_fix_docs.patch
</pre><p>
on top of sage-4.7.2.alpha3-prerelease.
</p>
<p>
One remark: It happened to me recently that I tried to qimport a patch from trac, but my university had a proxy, and for some reason it thought that the patch is cached. I therefore switched the cache off, for the computer in my office. That is where I test the patches.
</p>
<p>
But Burcin just told me that they have a proxy here as well. So, could it be that you tried to download the latest patch version, but your proxy only provided you with a cached but outdated version of the patches?
</p>
<p>
The other possibility is that I thought I have posted the patch version that I have on my computer in my office, but in fact posted an outdated version that I have on my netbook here. Testing it now.
</p>
TicketSimonKingFri, 30 Sep 2011 09:34:11 GMT
https://trac.sagemath.org/ticket/4539#comment:89
https://trac.sagemath.org/ticket/4539#comment:89
<p>
See <a class="new ticket" href="https://trac.sagemath.org/ticket/11878" title="enhancement: Proper implementation of quotients of g-algebras and polynomial rings (new)">#11878</a> for quotients of g-algebras.
</p>
TicketAlexanderDreyerFri, 30 Sep 2011 10:44:29 GMT
https://trac.sagemath.org/ticket/4539#comment:90
https://trac.sagemath.org/ticket/4539#comment:90
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:88" title="Comment 88">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I can not confirm Alexander's statement that some patch does not apply.
</p>
</blockquote>
<p>
Yeah, I just mixed up the order of the following patches:
</p>
<blockquote class="citation">
<p>
trac4539_pickling.patch
trac4539_normal_forms.patch
</p>
</blockquote>
<p>
Already the first didn't apply cleanly, but I overlooked. It built fine and the tests are running.
</p>
TicketSimonKingFri, 30 Sep 2011 12:06:54 GMTstatus changed
https://trac.sagemath.org/ticket/4539#comment:91
https://trac.sagemath.org/ticket/4539#comment:91
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_info</em>
</li>
</ul>
<p>
I found that the function <code>new_NRing</code>, that is supposed to return a valid <code>NCPolynomialRing_plural</code> out of a ring wrap, is broken. Important data, namely the matrices c and d, are left as <code>None</code>. Hence, for a ring produced with <code>new_NRing</code>, pickling won't work at all.
</p>
<p>
The question is whether we can leave it broken for now, and fix it separately, or leave this ticket open and "needs work". How shall we proceed?
</p>
TicketSimonKingFri, 30 Sep 2011 12:08:47 GMT
https://trac.sagemath.org/ticket/4539#comment:92
https://trac.sagemath.org/ticket/4539#comment:92
<p>
PS: Even <em>if</em> it returns a valid picklable ring, uniqueness of parents would break. So, we should analyse whether <code>new_NRing</code> is used in a critical (uniqueness-breaking) way in the current patch.
</p>
TicketSimonKingFri, 30 Sep 2011 12:12:09 GMT
https://trac.sagemath.org/ticket/4539#comment:93
https://trac.sagemath.org/ticket/4539#comment:93
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:92" title="Comment 92">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
PS: Even <em>if</em> it returns a valid picklable ring, uniqueness of parents would break. So, we should analyse whether <code>new_NRing</code> is used in a critical (uniqueness-breaking) way in the current patch.
</p>
</blockquote>
<p>
It concerncs the <code>SCA</code> function and the original approach towards quotients.
</p>
TicketSimonKingFri, 30 Sep 2011 14:44:01 GMTstatus changed
https://trac.sagemath.org/ticket/4539#comment:94
https://trac.sagemath.org/ticket/4539#comment:94
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
</ul>
<p>
I suggest that we leave stuff as it is now, so that it can be merged and we can build on top of it. Therefore, I revert it to "needs review".
</p>
<p>
If I am not mistaken, <code>SCA</code> is a special case in Singular anyway, and it is a huge difference whether one works in an <code>SCA</code> or in an isomorphic general g-algebra quotient (Oleksandr, could you tell how the ring should be created in this case? I guess the function <code>SCA</code> in the patch does not do the right thing).
</p>
<p>
I believe that there should be a sub-class of <code>NCPolynomialRing_plural</code> for general quotients of g-algebras, and then a sub-sub-class for SCA that uses the specialised implementation from Singular. But that should be on <a class="new ticket" href="https://trac.sagemath.org/ticket/11878" title="enhancement: Proper implementation of quotients of g-algebras and polynomial rings (new)">#11878</a>.
</p>
TicketOleksandrMotsakFri, 30 Sep 2011 15:05:17 GMT
https://trac.sagemath.org/ticket/4539#comment:95
https://trac.sagemath.org/ticket/4539#comment:95
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:94" title="Comment 94">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
If I am not mistaken, <code>SCA</code> is a special case in Singular anyway, and it is a huge difference whether one works in an <code>SCA</code> or in an isomorphic general g-algebra quotient (Oleksandr, could you tell how the ring should be created in this case? I guess the function <code>SCA</code> in the patch does not do the right thing).
</p>
</blockquote>
<p>
SCA structure is _autodetected_ upon creation of a GR-algebra (<code>qring</code>) in runtime. Therefore one should not use an extra method for this: just create a GRing and its quotient by correct twosided ideal there.
</p>
<p>
Test: if SCA implementation is used then <code>y*y == 0;</code> for each non-commutative (odd degree) variable <code>y</code>.
</p>
TicketAlexanderDreyerFri, 30 Sep 2011 21:50:51 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_fix_docs_32bit.patch</em>
</li>
</ul>
<p>
Fixed one doc test which faild in 32 bit
</p>
TicketAlexanderDreyerFri, 30 Sep 2011 21:58:06 GMTreviewer changed
https://trac.sagemath.org/ticket/4539#comment:96
https://trac.sagemath.org/ticket/4539#comment:96
<ul>
<li><strong>reviewer</strong>
changed from <em>Simon King</em> to <em>Simon King, Alexander Dreyer</em>
</li>
</ul>
<p>
One doctest failed on OSX 10.5 PPC. This is fixed in the attached patch. Since we postponed the quotient issue, I think I can give a positive review for Simon's work as well as for Michael's, Burcin's and Oleksandr's part , which I reviewed on SD24.
</p>
<p>
@Simon: If you accept my part we would have a positive review now.
</p>
TicketSimonKingSat, 01 Oct 2011 09:02:35 GMTstatus changed
https://trac.sagemath.org/ticket/4539#comment:97
https://trac.sagemath.org/ticket/4539#comment:97
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_info</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:96" title="Comment 96">AlexanderDreyer</a>:
</p>
<blockquote class="citation">
<p>
One doctest failed on OSX 10.5 PPC. This is fixed in the attached patch.
</p>
</blockquote>
<p>
It looks like this error already occurs <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a> - can you verify whether the error occurs on 32-bit with <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a>? Then it might be better to post your patch there.
</p>
<blockquote class="citation">
<p>
@Simon: If you accept my part we would have a positive review now.
</p>
</blockquote>
<p>
The "big" patch merely combines work of y'all, and I certainly give the stuff there a positive review. However, the question on <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a> should first be answered.
</p>
TicketAlexanderDreyerSat, 01 Oct 2011 22:28:03 GMT
https://trac.sagemath.org/ticket/4539#comment:98
https://trac.sagemath.org/ticket/4539#comment:98
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:97" title="Comment 97">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:96" title="Comment 96">AlexanderDreyer</a>:
</p>
<blockquote class="citation">
<p>
One doctest failed on OSX 10.5 PPC. This is fixed in the attached patch.
</p>
</blockquote>
<p>
It looks like this error already occurs <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a> - can you verify whether the error occurs on 32-bit with <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a>? Then it might be better to post your patch there.
</p>
</blockquote>
<p>
Indeed, I already had to apply <a class="ext-link" href="http://trac.sagemath.org/sage_trac/attachment/ticket/4539/trac4539_fix_docs_32bit.patch"><span class="icon"></span>http://trac.sagemath.org/sage_trac/attachment/ticket/4539/trac4539_fix_docs_32bit.patch</a> to <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a> on 32-bit systems.
</p>
TicketSimonKingSun, 02 Oct 2011 06:38:31 GMT
https://trac.sagemath.org/ticket/4539#comment:99
https://trac.sagemath.org/ticket/4539#comment:99
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:98" title="Comment 98">AlexanderDreyer</a>:
</p>
<blockquote class="citation">
<p>
Indeed, I already had to apply <a class="ext-link" href="http://trac.sagemath.org/sage_trac/attachment/ticket/4539/trac4539_fix_docs_32bit.patch"><span class="icon"></span>http://trac.sagemath.org/sage_trac/attachment/ticket/4539/trac4539_fix_docs_32bit.patch</a> to <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a> on 32-bit systems.
</p>
</blockquote>
<p>
OK, then <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/trac4539_fix_docs_32bit.patch" title="Attachment 'trac4539_fix_docs_32bit.patch' in Ticket #4539">trac4539_fix_docs_32bit.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/trac4539_fix_docs_32bit.patch" title="Download"></a> should better be moved to <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a> - since it only concerns doctests in the obvious way, but does not change the code, I think that your patch can be a reviewer patch for <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a>, thus, preserving the positive review that Martin gave (but then add your name in the "Reviewer" field of <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a>).
</p>
<p>
If that's done, then I'll try the stuff from here again, and then hopefully it can be turned into a positive review.
</p>
TicketAlexanderDreyerSun, 02 Oct 2011 21:23:30 GMTstatus changed
https://trac.sagemath.org/ticket/4539#comment:100
https://trac.sagemath.org/ticket/4539#comment:100
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
</ul>
TicketAlexanderDreyerSun, 02 Oct 2011 21:25:30 GMTstatus changed
https://trac.sagemath.org/ticket/4539#comment:101
https://trac.sagemath.org/ticket/4539#comment:101
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Now 32 bit issue was solved in <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a> (and has positive review again).
</p>
TicketjdemeyerTue, 04 Oct 2011 08:05:31 GMTmilestone changed
https://trac.sagemath.org/ticket/4539#comment:102
https://trac.sagemath.org/ticket/4539#comment:102
<ul>
<li><strong>milestone</strong>
changed from <em>sage-4.7.2</em> to <em>sage-4.7.3</em>
</li>
</ul>
TicketSimonKingTue, 04 Oct 2011 17:46:15 GMTstatus changed; work_issues set
https://trac.sagemath.org/ticket/4539#comment:103
https://trac.sagemath.org/ticket/4539#comment:103
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
<li><strong>work_issues</strong>
set to <em>Rebase wrt 4.7.3.alpha3 and #10903</em>
</li>
</ul>
<p>
Too bad. I was asked to rebase <a class="closed ticket" href="https://trac.sagemath.org/ticket/11586" title="defect: bug in p-adic extension norm method (closed: fixed)">#11586</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/11068" title="enhancement: Basic implementation of one- and twosided ideals of non-commutative ... (closed: fixed)">#11068</a> with respect to <a class="closed ticket" href="https://trac.sagemath.org/ticket/11339" title="defect: Refcounting for Singular rings (closed: fixed)">#11339</a>/<a class="closed ticket" href="https://trac.sagemath.org/ticket/10903" title="defect: Update Singular to 3-1-3-3 (closed: fixed)">#10903</a>. By consequence, the patches from here need to be rebase as well. Needs work...
</p>
TicketSimonKingTue, 04 Oct 2011 18:17:11 GMT
https://trac.sagemath.org/ticket/4539#comment:104
https://trac.sagemath.org/ticket/4539#comment:104
<p>
It is not going to be easy. <a class="closed ticket" href="https://trac.sagemath.org/ticket/11339" title="defect: Refcounting for Singular rings (closed: fixed)">#11339</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/10903" title="defect: Update Singular to 3-1-3-3 (closed: fixed)">#10903</a> have removed some functions (eg., <code>n_IsOne</code>) that are needed here. So, I need to find out where it was imported from.
</p>
TicketmalbTue, 04 Oct 2011 18:25:58 GMT
https://trac.sagemath.org/ticket/4539#comment:105
https://trac.sagemath.org/ticket/4539#comment:105
<p>
<code>n_IsOne</code> was replaced by <code>ring.cf.nIsOne(foo)</code>. We didn't remove any functionality, only replaced it by calls which are more explicit about the ring. If in doubt just ask :)
</p>
TicketSimonKingTue, 04 Oct 2011 18:31:07 GMT
https://trac.sagemath.org/ticket/4539#comment:106
https://trac.sagemath.org/ticket/4539#comment:106
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:105" title="Comment 105">malb</a>:
</p>
<blockquote class="citation">
<p>
<code>n_IsOne</code> was replaced by <code>ring.cf.nIsOne(foo)</code>. We didn't remove any functionality, only replaced it by calls which are more explicit about the ring. If in doubt just ask :)
</p>
</blockquote>
<p>
Yep, I already found the replacement (by doing a grep for <code>n_IsOne</code> in my <code>.hg/patches</code>, when I wanted to find out where that function came from).
</p>
TicketSimonKingWed, 05 Oct 2011 08:04:06 GMT
https://trac.sagemath.org/ticket/4539#comment:107
https://trac.sagemath.org/ticket/4539#comment:107
<p>
I didn't post my rebased patches yet, since I need to fix a few doctest errors.
</p>
<p>
Actually, the first error is a clear improvement. We have the following doctest:
</p>
<pre class="wiki"> sage: A.<x,y,z> = FreeAlgebra(QQ, 3)
sage: H = A.g_algebra({y*x:x*y-z, z*x:x*z+2*x, z*y:y*z-2*y})
sage: H.inject_variables()
Defining x, y, z
sage: I = H.ideal([y^2, x^2, z^2-H.one_element()],coerce=False)
sage: G = vector(I.gens()); G
d...: UserWarning: You are constructing a free module
over a noncommutative ring. Sage does not have a concept
of left/right and both sided modules, so be careful.
It's also not guaranteed that all multiplications are
done from the right side.
d...: UserWarning: You are constructing a free module
over a noncommutative ring. Sage does not have a concept
of left/right and both sided modules, so be careful.
It's also not guaranteed that all multiplications are
done from the right side.
(y^2, x^2, z^2 - 1)
sage: M = I.syzygy_module()
</pre><p>
With <a class="closed ticket" href="https://trac.sagemath.org/ticket/10903" title="defect: Update Singular to 3-1-3-3 (closed: fixed)">#10903</a> applied, one gets 9 Syzygies:
</p>
<pre class="wiki">sage: M[0]
(-z^2 - 8*z - 15, 0, y^2)
sage: M[1]
(0, -z^2 + 8*z - 15, x^2)
sage: M[2]
(x^2*z^2 + 8*x^2*z + 15*x^2, -y^2*z^2 + 8*y^2*z - 15*y^2, -4*x*y*z + 2*z^2 + 2*z)
sage: M[3]
(x^2*y*z^2 + 9*x^2*y*z - 6*x*z^3 + 20*x^2*y - 72*x*z^2 - 282*x*z - 360*x, -y^3*z^2 + 7*y^3*z - 12*y^3, 6*y*z^2)
sage: M[4]
(x^3*z^2 + 7*x^3*z + 12*x^3, -x*y^2*z^2 + 9*x*y^2*z - 4*y*z^3 - 20*x*y^2 + 52*y*z^2 - 224*y*z + 320*y, -6*x*z^2)
sage: M[5]
(x^2*y^2*z + 4*x^2*y^2 - 8*x*y*z^2 - 48*x*y*z + 12*z^3 - 64*x*y + 108*z^2 + 312*z + 288, -y^4*z + 4*y^4, 0)
sage: M[6]
(2*x^3*y*z + 8*x^3*y + 9*x^2*z + 27*x^2, -2*x*y^3*z + 8*x*y^3 - 12*y^2*z^2 + 99*y^2*z - 195*y^2, -36*x*y*z + 24*z^2 + 18*z)
sage: M[7]
(x^4*z + 4*x^4, -x^2*y^2*z + 4*x^2*y^2 - 4*x*y*z^2 + 32*x*y*z - 6*z^3 - 64*x*y + 66*z^2 - 240*z + 288, 0)
sage: M[8]
(x^3*y^2*z + 4*x^3*y^2 + 18*x^2*y*z - 36*x*z^3 + 66*x^2*y - 432*x*z^2 - 1656*x*z - 2052*x, -x*y^4*z + 4*x*y^4 - 8*y^3*z^2 + 62*y^3*z - 114*y^3, 48*y*z^2 - 36*y*z)
sage: M[9]
Traceback (most recent call last):
...
IndexError: matrix index out of range
</pre><p>
However, without <a class="closed ticket" href="https://trac.sagemath.org/ticket/10903" title="defect: Update Singular to 3-1-3-3 (closed: fixed)">#10903</a> (and with the original patches applied), one gets what is expected in the doc tests, namely 10 Syzygies -- but two of them are identical:
</p>
<pre class="wiki">sage: M[0]
(-z^2 - 8*z - 15, 0, y^2)
sage: M[1]
(0, -z^2 + 8*z - 15, x^2)
sage: M[2]
(x^2*z^2 + 8*x^2*z + 15*x^2, -y^2*z^2 + 8*y^2*z - 15*y^2, -4*x*y*z + 2*z^2 + 2*z)
sage: M[3]
(x^2*y*z^2 + 9*x^2*y*z - 6*x*z^3 + 20*x^2*y - 72*x*z^2 - 282*x*z - 360*x, -y^3*z^2 + 7*y^3*z - 12*y^3, 6*y*z^2)
sage: M[4]
(x^3*z^2 + 7*x^3*z + 12*x^3, -x*y^2*z^2 + 9*x*y^2*z - 4*y*z^3 - 20*x*y^2 + 52*y*z^2 - 224*y*z + 320*y, -6*x*z^2)
sage: M[5]
(x^2*y^2*z + 4*x^2*y^2 - 8*x*y*z^2 - 48*x*y*z + 12*z^3 - 64*x*y + 108*z^2 + 312*z + 288, -y^4*z + 4*y^4, 0)
sage: M[6]
(2*x^3*y*z + 8*x^3*y + 9*x^2*z + 27*x^2, -2*x*y^3*z + 8*x*y^3 - 12*y^2*z^2 + 99*y^2*z - 195*y^2, -36*x*y*z + 24*z^2 + 18*z)
sage: M[7]
(2*x^3*y*z + 8*x^3*y + 9*x^2*z + 27*x^2, -2*x*y^3*z + 8*x*y^3 - 12*y^2*z^2 + 99*y^2*z - 195*y^2, -36*x*y*z + 24*z^2 + 18*z)
sage: M[8]
(x^4*z + 4*x^4, -x^2*y^2*z + 4*x^2*y^2 - 4*x*y*z^2 + 32*x*y*z - 6*z^3 - 64*x*y + 66*z^2 - 240*z + 288, 0)
sage: M[9]
(x^3*y^2*z + 4*x^3*y^2 + 18*x^2*y*z - 36*x*z^3 + 66*x^2*y - 432*x*z^2 - 1656*x*z - 2052*x, -x*y^4*z + 4*x*y^4 - 8*y^3*z^2 + 62*y^3*z - 114*y^3, 48*y*z^2 - 36*y*z)
sage: M[7]==M[6]
True
</pre><p>
So, the old Singular version forgot to remove a redundant Syzygy.
</p>
TicketSimonKingWed, 05 Oct 2011 08:12:32 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_libplural_rel10903.patch</em>
</li>
</ul>
<p>
Combined patch relative to sage-4.7.2.alpha3 plus <a class="closed ticket" href="https://trac.sagemath.org/ticket/11068" title="enhancement: Basic implementation of one- and twosided ideals of non-commutative ... (closed: fixed)">#11068</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/11856" title="defect: Raise an overflow error if the exponent of a multivariate polynomial ... (closed: fixed)">#11856</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/10903" title="defect: Update Singular to 3-1-3-3 (closed: fixed)">#10903</a>
</p>
TicketSimonKingWed, 05 Oct 2011 08:13:10 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_pickling_rel10903.patch</em>
</li>
</ul>
<p>
Pickling of nc rings and polynomials, rel <a class="closed ticket" href="https://trac.sagemath.org/ticket/10903" title="defect: Update Singular to 3-1-3-3 (closed: fixed)">#10903</a>
</p>
TicketSimonKingWed, 05 Oct 2011 08:13:40 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_normal_forms_rel10903.patch</em>
</li>
</ul>
<p>
Normal forms, quotient rings, and ideal containment, rel <a class="closed ticket" href="https://trac.sagemath.org/ticket/10903" title="defect: Update Singular to 3-1-3-3 (closed: fixed)">#10903</a>
</p>
TicketSimonKingWed, 05 Oct 2011 08:20:46 GMTstatus, dependencies, description changed; work_issues deleted
https://trac.sagemath.org/ticket/4539#comment:108
https://trac.sagemath.org/ticket/4539#comment:108
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>dependencies</strong>
changed from <em>#11068 #11316 #11856</em> to <em>#11068 #11316 #11856 #10903</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/4539?action=diff&version=108">diff</a>)
</li>
<li><strong>work_issues</strong>
<em>Rebase wrt 4.7.3.alpha3 and #10903</em> deleted
</li>
</ul>
<p>
Hoorray! The other doctest error was even easier to fix: It has been a new test, and I simply had a typo in it.
</p>
<p>
Because of <a class="closed ticket" href="https://trac.sagemath.org/ticket/11339" title="defect: Refcounting for Singular rings (closed: fixed)">#11339</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/10903" title="defect: Update Singular to 3-1-3-3 (closed: fixed)">#10903</a>, I had to change some lines in the code. In order to make the changes more easily visible, I attached the new patches under a new name, so that you can compare them with the old patches.
</p>
<p>
Could you please have a look whether we can return to the positive review?
</p>
<p>
Apply trac4539_libplural_rel10903.patch trac4539_pickling_rel10903.patch trac4539_normal_forms_rel10903.patch trac4539_fix_docs_rel10903.patch
</p>
TicketAlexanderDreyerWed, 05 Oct 2011 09:02:00 GMT
https://trac.sagemath.org/ticket/4539#comment:109
https://trac.sagemath.org/ticket/4539#comment:109
<p>
The patches look sane. I'll test them now (setting up 4.7.2alph3 will last some time).
</p>
TicketSimonKingWed, 05 Oct 2011 09:04:35 GMT
https://trac.sagemath.org/ticket/4539#comment:110
https://trac.sagemath.org/ticket/4539#comment:110
<p>
FWIW, all tests pass for me (starting with the "official version" of sage-4.7.3.alpha3).
</p>
TicketAlexanderDreyerWed, 05 Oct 2011 09:13:15 GMT
https://trac.sagemath.org/ticket/4539#comment:111
https://trac.sagemath.org/ticket/4539#comment:111
<p>
Can you just post the output of <code>hg qapplied</code>? this would simplify things for me.
</p>
TicketSimonKingWed, 05 Oct 2011 09:19:23 GMT
https://trac.sagemath.org/ticket/4539#comment:112
https://trac.sagemath.org/ticket/4539#comment:112
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:111" title="Comment 111">AlexanderDreyer</a>:
</p>
<blockquote class="citation">
<p>
Can you just post the output of <code>hg qapplied</code>? this would simplify things for me.
</p>
</blockquote>
<p>
Starting with sage-4.7.2.alpha3 (no prerelease this time):
</p>
<pre class="wiki">$ hg qapplied
trac_11339_refcount_singular_rings.patch
trac_11339_refcount_singular_polynomials.patch
trac_10903_singular-3-1-3-3.patch
trac_10903_singular-fixes.patch
trac11856_exponent_overflow.patch
trac11856_fix_docs_32bit.patch
trac11115-cached_cython.patch
trac11115_element_with_cache.patch
trac11115_cached_function_pickling.patch
trac_11115_reviewer.patch
trac11068_nc_ideals_and_quotients.patch
trac11068_quotient_ring_without_names.patch
trac11068_lifting_map.patch
trac4539_libplural_rel10903.patch
trac4539_pickling_rel10903.patch
trac4539_normal_forms_rel10903.patch
trac4539_fix_docs_rel10903.patch
</pre>
TicketSimonKingWed, 05 Oct 2011 10:36:10 GMT
https://trac.sagemath.org/ticket/4539#comment:113
https://trac.sagemath.org/ticket/4539#comment:113
<p>
Sorry, I had to produce a new version of the last patch: The first patch has introduced a wrong instance of the <code>..todo::</code> markup. The docbuilder complained about it. Since <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/trac4539_fix_docs_rel10903.patch" title="Attachment 'trac4539_fix_docs_rel10903.patch' in Ticket #4539">trac4539_fix_docs_rel10903.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/trac4539_fix_docs_rel10903.patch" title="Download"></a> is responsible for fixing the doc strings, I fixed it there.
</p>
<p>
So, please replace the last patch with the new version, and also try to build the documentation (<code>sage -docbuild reference html</code>).
</p>
TicketSimonKingWed, 05 Oct 2011 10:40:23 GMTstatus changed
https://trac.sagemath.org/ticket/4539#comment:114
https://trac.sagemath.org/ticket/4539#comment:114
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
OMG. It seems that I managed to destroy the patch that I have just attached by a patch that I had prepared for <a class="closed ticket" href="https://trac.sagemath.org/ticket/10903" title="defect: Update Singular to 3-1-3-3 (closed: fixed)">#10903</a>. Needs work, for now.
</p>
TicketSimonKingWed, 05 Oct 2011 10:53:00 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>8800_functor_pushout_doc_and_fixes.patch</em>
</li>
</ul>
<p>
Fixing doc strings and doc tests , rel <a class="closed ticket" href="https://trac.sagemath.org/ticket/10903" title="defect: Update Singular to 3-1-3-3 (closed: fixed)">#10903</a>
</p>
TicketSimonKingWed, 05 Oct 2011 10:53:53 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_fix_docs_rel10903.2.patch</em>
</li>
</ul>
<p>
Fixing doc strings and doc tests , rel <a class="closed ticket" href="https://trac.sagemath.org/ticket/10903" title="defect: Update Singular to 3-1-3-3 (closed: fixed)">#10903</a>
</p>
TicketSimonKingWed, 05 Oct 2011 10:54:11 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_fix_docs_rel10903.patch</em>
</li>
</ul>
<p>
Fixing doc strings and doc tests , rel <a class="closed ticket" href="https://trac.sagemath.org/ticket/10903" title="defect: Update Singular to 3-1-3-3 (closed: fixed)">#10903</a>
</p>
TicketSimonKingWed, 05 Oct 2011 10:57:59 GMTstatus changed
https://trac.sagemath.org/ticket/4539#comment:115
https://trac.sagemath.org/ticket/4539#comment:115
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
I urgently need a break. It is unbelievable how many errors I made in the past 30 minutes.
</p>
<p>
Anyway.
</p>
<p>
I have now updated the patch (after first attaching a wrong file, followed by the correct file under a wrong name, and those things).
</p>
<p>
Note that there is now a new patch at <a class="closed ticket" href="https://trac.sagemath.org/ticket/10903" title="defect: Update Singular to 3-1-3-3 (closed: fixed)">#10903</a> - without that patch, building the documentation would fail.
</p>
<p>
The new version of <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/trac4539_fix_docs_rel10903.patch" title="Attachment 'trac4539_fix_docs_rel10903.patch' in Ticket #4539">trac4539_fix_docs_rel10903.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/trac4539_fix_docs_rel10903.patch" title="Download"></a> fixes one wrongly formatted <code>.. todo::</code> directive.
</p>
<p>
Please test whether the documentation builds fine for you.
</p>
<p>
Apply trac4539_libplural_rel10903.patch trac4539_pickling_rel10903.patch trac4539_normal_forms_rel10903.patch trac4539_fix_docs_rel10903.patch
</p>
TicketAlexanderDreyerWed, 05 Oct 2011 18:18:18 GMT
https://trac.sagemath.org/ticket/4539#comment:116
https://trac.sagemath.org/ticket/4539#comment:116
<p>
Everything is fine, but one issue: Unfortunately the docbuild contains one uncaught exception (on SuSE 11):
</p>
<pre class="wiki">sphinx-build -b html -d /p/sys/Sage/share/versions/sage-4.7.2.alpha3/devel/sage/doc/output/doctrees/en/reference /p/sys/Sage/share/versions/sage-4.7.2.alpha3/devel/sage/doc/en/reference /p/sys/Sage/share/versions/sage-4.7.2.alpha3/devel/sage/doc/output/html/en/reference
Running Sphinx v1.0.4
loading pickled environment... done
building [html]: targets for 152 source files that are out of date
updating environment: 1 added, 152 changed, 0 removed
reading sources... [ 99%] sage/symbolic/expression
Exception occurred:
File "/p/sys/Sage/share/versions/sage-4.7.2.alpha3/devel/sage/doc/common/conf.py", line 378, in skip_member
if (hasattr(obj, '__name__') and obj.__name__.find('.') != -1 and
AttributeError: 'NoneType' object has no attribute 'find'
The full traceback has been saved in /tmp/sphinx-err-RJnoHz.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
Either send bugs to the mailing list at <http://groups.google.com/group/sphinx-dev/>,
or report them in the tracker at <http://bitbucket.org/birkenfeld/sphinx/issues/>. Thanks!
Build finished. The built documents can be found in /p/sys/Sage/share/versions/sage-4.7.2.alpha3/devel/sage/doc/output/html/en/reference
</pre><p>
I suggested the reviewer patch: <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/4539/trac4539_docbuild_reviewer.patch" title="Attachment 'trac4539_docbuild_reviewer.patch' in Ticket #4539">trac4539_docbuild_reviewer.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/4539/trac4539_docbuild_reviewer.patch" title="Download"></a>
</p>
<p>
With that patch we are close to a positive review: I'm also running tests on OS X.
</p>
TicketAlexanderDreyerWed, 05 Oct 2011 18:18:58 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_docbuild_reviewer.patch</em>
</li>
</ul>
TicketSimonKingWed, 05 Oct 2011 18:43:04 GMT
https://trac.sagemath.org/ticket/4539#comment:117
https://trac.sagemath.org/ticket/4539#comment:117
<p>
Hi Alexander,
</p>
<p>
Note that there is a new patch at <a class="closed ticket" href="https://trac.sagemath.org/ticket/10903" title="defect: Update Singular to 3-1-3-3 (closed: fixed)">#10903</a> (where the docbuild crash was introduced), and it fixes the problem in a more satisfying way. The problem was that under certain circumstances the name of a deprecated Cython method could not be determined - but with the new patch from <a class="closed ticket" href="https://trac.sagemath.org/ticket/10903" title="defect: Update Singular to 3-1-3-3 (closed: fixed)">#10903</a> (actually there are TWO new patches) the problem is fixed.
</p>
<p>
So, the docbuild reviewer patch is not needed.
</p>
TicketAlexanderDreyerThu, 06 Oct 2011 07:59:16 GMTstatus changed
https://trac.sagemath.org/ticket/4539#comment:118
https://trac.sagemath.org/ticket/4539#comment:118
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Building, installing and testing succeeded on SuSE 11 Enterprise amd64 and OS X 10.5 ppc. So we can switch back to positive review.
</p>
TicketjdemeyerTue, 18 Oct 2011 20:16:01 GMTmilestone changed
https://trac.sagemath.org/ticket/4539#comment:119
https://trac.sagemath.org/ticket/4539#comment:119
<ul>
<li><strong>milestone</strong>
changed from <em>sage-4.7.3</em> to <em>sage-pending</em>
</li>
</ul>
TicketjdemeyerWed, 02 Nov 2011 13:58:40 GMTdependencies changed
https://trac.sagemath.org/ticket/4539#comment:120
https://trac.sagemath.org/ticket/4539#comment:120
<ul>
<li><strong>dependencies</strong>
changed from <em>#11068 #11316 #11856 #10903</em> to <em>#11316, #11856, #10903, #9138, #11900, #11115, #11068</em>
</li>
</ul>
TicketjdemeyerSat, 05 Nov 2011 14:01:46 GMTattachment set
https://trac.sagemath.org/ticket/4539
https://trac.sagemath.org/ticket/4539
<ul>
<li><strong>attachment</strong>
set to <em>trac4539_libplural_rel11761.patch</em>
</li>
</ul>
TicketjdemeyerSat, 05 Nov 2011 14:05:43 GMTdependencies, description changed
https://trac.sagemath.org/ticket/4539#comment:121
https://trac.sagemath.org/ticket/4539#comment:121
<ul>
<li><strong>dependencies</strong>
changed from <em>#11316, #11856, #10903, #9138, #11900, #11115, #11068</em> to <em>#11316, #11856, #10903, #9138, #11900, #11115, #11068, #11761</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/4539?action=diff&version=121">diff</a>)
</li>
</ul>
TicketSimonKingTue, 08 Nov 2011 15:15:44 GMTstatus changed
https://trac.sagemath.org/ticket/4539#comment:122
https://trac.sagemath.org/ticket/4539#comment:122
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_info</em>
</li>
</ul>
<p>
It could be that we need some work here. The first patch does not apply when we start with sage-4.8.alpha0. Namely, in sage/libs/singular/function.pyx, it expects the line
</p>
<pre class="wiki"> ring2 = None
</pre><p>
but this line has been removed. I don't know in which ticket that has happened. By consequence, a rather complicated chunk of the patch does not apply.
</p>
<p>
What shall we do? In order to avoid premature work, it would be an option to wait until finally all the dependencies got a positive review.
</p>
TicketAlexanderDreyerTue, 08 Nov 2011 22:34:16 GMT
https://trac.sagemath.org/ticket/4539#comment:123
https://trac.sagemath.org/ticket/4539#comment:123
<p>
That line comes from <a class="ext-link" href="http://trac.sagemath.org/sage_trac/attachment/ticket/11761/11761-cython-0.15.patch"><span class="icon"></span>http://trac.sagemath.org/sage_trac/attachment/ticket/11761/11761-cython-0.15.patch</a> (Cython is more strict here.) That patch was not merged in alpha0.
</p>
TicketSimonKingTue, 08 Nov 2011 23:00:01 GMTstatus changed
https://trac.sagemath.org/ticket/4539#comment:124
https://trac.sagemath.org/ticket/4539#comment:124
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/4539#comment:123" title="Comment 123">AlexanderDreyer</a>:
</p>
<blockquote class="citation">
<p>
That line comes from <a class="ext-link" href="http://trac.sagemath.org/sage_trac/attachment/ticket/11761/11761-cython-0.15.patch"><span class="icon"></span>http://trac.sagemath.org/sage_trac/attachment/ticket/11761/11761-cython-0.15.patch</a> (Cython is more strict here.)
</p>
</blockquote>
<p>
Yes, now I see it: <a class="closed ticket" href="https://trac.sagemath.org/ticket/11761" title="enhancement: Upgrade Cython to 0.15.1 (closed: fixed)">#11761</a> is a dependency! So, sorry for the noise, and back to a positive review - which needs two steps. One...
</p>
TicketSimonKingTue, 08 Nov 2011 23:00:31 GMTstatus changed
https://trac.sagemath.org/ticket/4539#comment:125
https://trac.sagemath.org/ticket/4539#comment:125
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
... and two.
</p>
TicketjdemeyerFri, 09 Dec 2011 23:57:43 GMTmilestone changed
https://trac.sagemath.org/ticket/4539#comment:126
https://trac.sagemath.org/ticket/4539#comment:126
<ul>
<li><strong>milestone</strong>
changed from <em>sage-pending</em> to <em>sage-5.0</em>
</li>
</ul>
TicketjdemeyerSat, 21 Jan 2012 23:39:13 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/4539#comment:127
https://trac.sagemath.org/ticket/4539#comment:127
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>merged</strong>
set to <em>sage-5.0.beta1</em>
</li>
</ul>
Ticket