sagemath / sage

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

bug in determinant of Matrix_mpolynomial_dense #23535

Closed videlec closed 7 years ago

videlec commented 7 years ago
sage: K.<a,b> = NumberField([x^2-2,x^2-5])
sage: R.<s,t> = K[]
sage: matrix(R,4, [1,a,s,t]*4).det()
Traceback (most recent call last):
.../matrix/matrix2.pyx
in sage.matrix.matrix2.Matrix.det
(build/cythonized/sage/matrix/matrix2.c:17532)()
   1869             6
   1870         """
-> 1871         return self.determinant(*args, **kwds)
   1872 
   1873     def __abs__(self):

.../matrix/matrix_mpolynomial_dense.pyx
in sage.matrix.matrix_mpolynomial_dense.Matrix_mpolynomial_dense.determinant 
(build/cythonized/sage/matrix/matrix_mpolynomial_dense.cpp:7193)()
    547 
    548             elif can_convert_to_singular(self.base_ring()):
--> 549                 d = R(self._singular_().det())
    550 
    551             else:

.../matrix/matrix1.pyx
in sage.matrix.matrix1.Matrix._singular_
(build/cythonized/sage/matrix/matrix1.c:4778)()
    372             singular = singular_default
    373         try:
--> 374             self.base_ring()._singular_(singular)
    375         except (NotImplementedError, AttributeError):
    376             raise TypeError("Cannot coerce to Singular")

.../rings/polynomial/polynomial_singular_interface.pyc
in _singular_(self, singular)
    217             return R
    218         except (AttributeError, ValueError):
--> 219             return self._singular_init_(singular)
    220 
    221     def _singular_init_(self, singular=singular):

.../rings/polynomial/polynomial_singular_interface.pyc
in _singular_init_(self, singular)
    333             self.__singular = singular.ring("(integer)", _vars, order=order, check=False)
    334         else:
--> 335             raise TypeError("no conversion to a Singular ring defined")
    336 
    337         return self.__singular

TypeError: no conversion to a Singular ring defined

The function can_convert_to_singular is to blame

sage: from sage.rings.polynomial.polynomial_singular_interface import can_convert_to_singular
sage: K.<a,b> = NumberField([x^2-2,x^2-5])
sage: can_convert_to_singular(K)
True
sage: K._singular_()
** BOOM **

Original report on this question on ask.

Component: algebra

Keywords: bug

Author: Vincent Delecroix

Branch/Commit: 5e23e17

Reviewer: Marc Mezzarobba

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

videlec commented 7 years ago

Description changed:

--- 
+++ 
@@ -50,4 +50,17 @@
 TypeError: no conversion to a Singular ring defined

+The function can_convert_to_singular is to blame + + +sage: from sage.rings.polynomial.polynomial_singular_interface import can_convert_to_singular +sage: K.<s,t> = NumberField([x^2-2,x^2-5]) +sage: can_convert_to_singular(K) +True +sage: K._singular_() +** BOOM ** + + +--- + Original report on this question on ask.

videlec commented 7 years ago

Description changed:

--- 
+++ 
@@ -54,7 +54,7 @@

sage: from sage.rings.polynomial.polynomial_singular_interface import can_convert_to_singular -sage: K.<s,t> = NumberField([x^2-2,x^2-5]) +sage: K.<a,b> = NumberField([x^2-2,x^2-5]) sage: can_convert_to_singular(K) True sage: K.singular()

videlec commented 7 years ago

Branch: u/vdelecroix/23535

videlec commented 7 years ago

New commits:

5e23e1723535: fix det for mpolynomial matrices
videlec commented 7 years ago

Commit: 5e23e17

videlec commented 7 years ago

Author: Vincent Delecroix

mezzarobba commented 7 years ago
comment:5

LGTM. Have you considered going to an absolute number field and calling Singular? Do you know if that could be better in practice?

mezzarobba commented 7 years ago

Reviewer: Marc Mezzarobba

videlec commented 7 years ago
comment:6

I believe it would be better. Everything is there (except that you have to chose an explicit name!)

sage: K.<a,b> = NumberField([x^2-2, x^2-5])
sage: L = K.absolute_field('c')
sage: L
Number Field in c with defining polynomial x^4 - 14*x^2 + 9
sage: L.convert_map_from(K)
Isomorphism map:
  From: Number Field in a with defining polynomial x^2 - 2 over its base field
  To:   Number Field in c with defining polynomial x^4 - 14*x^2 + 9
sage: K.convert_map_from(L)
Isomorphism map:
  From: Number Field in c with defining polynomial x^4 - 14*x^2 + 9
  To:   Number Field in a with defining polynomial x^2 - 2 over its base field

and then the following works

sage: K(random_matrix(K,4).change_ring(L).det())  # random
42

However I do not want to repeat this trick at every place singular is called in Sage! One possible way to do it is to enrich the ._singular_() method of elements. If we had a ._singular_morphisms_() on parent returning a pair (ToSingular, FromSingular) we could do

class MultiPolyMatrix:
    def determinant(self):
        try:
            to, from = self._singular_morphisms_()
        except ValueError:
            # no Singular
            # find something else
        else:
            return from(to(self).determinant())
vbraun commented 7 years ago

Changed branch from u/vdelecroix/23535 to 5e23e17