sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.48k stars 487 forks source link

Extend singular_function to non-commutative polynomial rings by refactoring of plural #11892

Open simon-king-jena opened 13 years ago

simon-king-jena commented 13 years ago

4539 provides g-algebras (non-commutative polynomial rings). However, sage.libs.singular.function.singular_function doesn't accept them as input.

Here is an example that works with usual polynomial rings

sage: from sage.libs.singular.function import singular_function
sage: P.<x,y,z> = QQ[]
sage: std = singular_function('std')
sage: NF = singular_function('NF')
sage: s_id = singular_function('ideal')
sage: NF(P.2^2,std(s_id(P(0))))
// ** _ is no standard basis
z^2

The same example fails with a non-commutative polynomial ring:

sage: A.<x,y,z> = FreeAlgebra(QQ, 3)
sage: H.<x,y,z> = A.g_algebra({z*x:x*z+2*x, z*y:y*z-2*y})
sage: NF(H.2^2,std(s_id(H(0))))
// ** _ is no standard basis
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)

/mnt/local/king/SAGE/debug/sage-4.7.2.alpha3-prerelease/devel/sage-main/<ipython console> in <module>()

/mnt/local/king/SAGE/debug/sage-4.7.2.alpha3-prerelease/local/lib/python2.6/site-packages/sage/libs/singular/function.so in sage.libs.singular.function.SingularFunction.__call__ (sage/libs/singular/function.cpp:11123)()
...
/mnt/local/king/SAGE/debug/sage-4.7.2.alpha3-prerelease/local/lib/python2.6/site-packages/sage/libs/singular/function.so in sage.libs.singular.function.call_function (sage/libs/singular/function.cpp:12682)()

/mnt/local/king/SAGE/debug/sage-4.7.2.alpha3-prerelease/local/lib/python2.6/site-packages/sage/libs/singular/function.so in sage.libs.singular.function.Converter.to_python (sage/libs/singular/function.cpp:9580)()

/mnt/local/king/SAGE/debug/sage-4.7.2.alpha3-prerelease/local/lib/python2.6/site-packages/sage/rings/polynomial/multi_polynomial_libsingular.so in sage.rings.polynomial.multi_polynomial_libsingular.MPolynomial_libsingular.__init__ (sage/rings/polynomial/multi_polynomial_libsingular.cpp:13943)()

TypeError: Argument 'parent' has incorrect type (expected sage.rings.polynomial.multi_polynomial_libsingular.MPolynomialRing_libsingular, got sage.rings.polynomial.plural.NCPolynomialRing_plural)

Apart from the misleading warning about the ideal otained from std not being a standard basis, I think the second example should work.

Two approaches: Add sage.rings.polynomial.plural.NCPolynomialRing_plural as a special case to sage.libs.singular.function, or make the non-commutative rings inherit from the commutative (or probably better the other way around), such that isinstance is happy.

Depends on #4539

CC: @saliola @mwhansen @alexanderdreyer @sagetrac-OleksandrMotsak @sagetrac-PolyBoRi @malb @burcin

Component: algebra

Issue created by migration from https://trac.sagemath.org/ticket/11892

simon-king-jena commented 13 years ago
comment:1

I forgot to copy-and-paste two lines from the example...

simon-king-jena commented 13 years ago

Description changed:

--- 
+++ 
@@ -15,6 +15,8 @@
 The same example fails with a non-commutative polynomial ring:

+sage: A.<x,y,z> = FreeAlgebra(QQ, 3) +sage: H.<x,y,z> = A.g_algebra({zx:xz+2x, zy:yz-2y}) sage: NF(H.2^2,std(sid(H(0)))) // ** is no standard basis

simon-king-jena commented 13 years ago
comment:2

It seems that the approach "make NCPolynomialRing_plural a subclass of MPolynomialRing_libsingular" is easy.

However, that change is so small that I tend to include it where I really need it: In #11878. So, this should be closed.

1998e663-c3b5-4cf6-92c3-7f1771ca5185 commented 13 years ago
comment:3

There is a function called is_sage_wrapper_for_singular_ring in the plural patch. The comment says that it's broken (for whatever reason). Shouldn't it be the right place for this check?

simon-king-jena commented 13 years ago
comment:4

Replying to @sagetrac-PolyBoRi:

There is a function called is_sage_wrapper_for_singular_ring in the plural patch. The comment says that it's broken (for whatever reason). Shouldn't it be the right place for this check?

Looking at the code, I don't understand why it doesn't work.

However, I have to think how to organise work. Things to do involve

These three "to-do"s are closely linked.

Do I understand correctly that generally it is better to have several smaller tickets, rather than one big ticket?

simon-king-jena commented 13 years ago
comment:5

I suggest to proceed as follows:

1998e663-c3b5-4cf6-92c3-7f1771ca5185 commented 11 years ago
comment:7

Sorry, I do not have the time to check anything in detail now. I was a little bit surprised that the ticket is still open. I am sure that the functionality worked before.

You can use a common base class, if you want. But inheriting the noncommutative ring from the commutative seems absolutely the wrong way to go. Still abstractions like is_sage_wrapper_for_singular_ring seem unavoidable to me. Thank for all your efforts (very appreciated) and my strong opinion without being able to dive into the topic again.

Cheers, Michael