Skip to content

Commit 90a1d21

Browse files
author
Release Manager
committed
gh-39451: Fixes incorrect behaviour when taking the derivative of a constant matrix <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes #12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes #12345". --> Fixes #15067. Fixed error in the _derivative methods of matrix_dense.pyx and matrix_sparse.pyx. These functions attempted to call the derivative method belonging to each element of the matrix. This caused incorrect behaviour when the element did not have a derivative method. This was fixed by altering the code to call the derivative function defined in /src/sage/calculus/functional.py with the element as a parameter, instead of the derivative method belonging to the element. Doctests were added to verify this change. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [X] The title is concise and informative. - [X] The description explains in detail what this PR is about. - [X] I have linked a relevant issue or discussion. - [X] I have created tests covering the changes. - [X] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - #12345: short description why this is a dependency --> <!-- - #34567: ... --> URL: #39451 Reported by: Caleb Van't Land Reviewer(s):
2 parents 882116c + 8cecf8c commit 90a1d21

File tree

2 files changed

+22
-2
lines changed

2 files changed

+22
-2
lines changed

src/sage/matrix/matrix_dense.pyx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ TESTS::
1111
cimport sage.matrix.matrix as matrix
1212

1313
from sage.structure.richcmp cimport richcmp_item, rich_to_bool
14+
from sage.calculus.functional import derivative
1415
import sage.matrix.matrix_space
1516
import sage.structure.sequence
1617

@@ -275,11 +276,19 @@ cdef class Matrix_dense(matrix.Matrix):
275276
sage: m._derivative(x) # needs sage.symbolic
276277
[ 0 1]
277278
[ 2*x 3*x^2]
279+
280+
TESTS:
281+
282+
Verify that :issue:`15067` is fixed::
283+
284+
sage: u = matrix(1, 2, [-1, 1])
285+
sage: derivative(u, x)
286+
[0 0]
278287
"""
279288
# We could just use apply_map
280289
if self._nrows==0 or self._ncols==0:
281290
return self.__copy__()
282-
v = [z.derivative(var) for z in self.list()]
291+
v = [sage.calculus.functional.derivative(z, var) for z in self.list()]
283292
if R is None:
284293
v = sage.structure.sequence.Sequence(v)
285294
R = v.universe()

src/sage/matrix/matrix_sparse.pyx

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ from cysignals.signals cimport sig_check
1616
cimport sage.matrix.matrix as matrix
1717
cimport sage.matrix.matrix0 as matrix0
1818
from sage.categories.rings import Rings
19+
from sage.calculus.functional import derivative
1920
from sage.structure.element cimport Element, Vector
2021
from sage.structure.richcmp cimport richcmp_item, rich_to_bool
2122

@@ -810,13 +811,23 @@ cdef class Matrix_sparse(matrix.Matrix):
810811
sage: m._derivative(x) # needs sage.symbolic
811812
[ 0 1]
812813
[ 2*x 3*x^2]
814+
815+
TESTS:
816+
817+
Verify that :issue:`15067` is fixed::
818+
819+
sage: m = matrix(3, 3, {(1, 1): 2, (0,2): 5})
820+
sage: derivative(m, x)
821+
[0 0 0]
822+
[0 0 0]
823+
[0 0 0]
813824
"""
814825
# We would just use apply_map, except that Cython does not
815826
# allow lambda functions
816827

817828
if self._nrows==0 or self._ncols==0:
818829
return self.__copy__()
819-
v = [(ij, z.derivative(var)) for ij, z in self.dict().iteritems()]
830+
v = [(ij, sage.calculus.functional.derivative(z, var)) for ij, z in self.dict().iteritems()]
820831
if R is None:
821832
w = [x for _, x in v]
822833
w = sage.structure.sequence.Sequence(w)

0 commit comments

Comments
 (0)