Deprecate block_diag from math module in favor of PyTensor
#7132
Conversation
….slinalg.block_diag
for more information, see https://pre-commit.ci
|
@larryshamalama, @ricardoV94, and @jessegrabowski please look into the PR and suggest if any changes are required. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #7132 +/- ##
===========================================
- Coverage 90.17% 39.96% -50.21%
===========================================
Files 101 101
Lines 16932 16928 -4
===========================================
- Hits 15269 6766 -8503
- Misses 1663 10162 +8499
|
|
What am I doing wrong here, the Codecov/patch test failed? |
|
@AryanNanda17 I asked the person commenting earlier on the original issue if they would be interested in working on this issue as a courtesy. If you see an issue you would like to work on where someone else seemed to be interested recently please confirm with them first if they are happy to let you take over. Our official policy is opening a PR first is what counts, but for these small non critical issues we would like to give room for everyone to have their first chance. There are many harder issues that we need help with for which there's no risk of multiple people being interested at the same time. |
|
@ricardoV94, Can You suggest some harder issues? |
|
These beginner friendly ones don't seem to have anyone interested and may be good?
We also have many good issue in the related repos: PyTensor and PyMC-Experimental that tend to fall more out of the radar: |
pymc/math.py
Outdated
| @@ -577,4 +528,8 @@ def block_diagonal(matrices, sparse=False, format="csr"): | |||
| """ | |||
| if len(matrices) == 1: # graph optimization | |||
| return matrices[0] | |||
| return BlockDiagonalMatrix(sparse=sparse, format=format)(*matrices) | |||
|
|
|||
There was a problem hiding this comment.
Can you add a FutureWarning mentioning this is deprecated in favor of the pytensor function, so we can remove this thin wrapper sometime in the future?
|
@ricardoV94, I made the changes please checkout. |
pymc/math.py
Outdated
| @@ -576,5 +527,13 @@ def block_diagonal(matrices, sparse=False, format="csr"): | |||
| matrix | |||
| """ | |||
| if len(matrices) == 1: # graph optimization | |||
| warnings.warn( | |||
There was a problem hiding this comment.
Any use of the function should issue a warning, not just this
pymc/math.py
Outdated
| warnings.warn( | ||
| "The behavior of block_diagonal when only one matrix is provided is deprecated. Use pytensor function instead.", | ||
| FutureWarning, | ||
| stacklevel=2, | ||
| ) |
There was a problem hiding this comment.
| warnings.warn( | |
| "The behavior of block_diagonal when only one matrix is provided is deprecated. Use pytensor function instead.", | |
| FutureWarning, | |
| stacklevel=2, | |
| ) | |
| warnings.warn( | |
| "`pymc.math.block_diagonal` is deprecated in favor of `pytensor.tensor.linalg.block_diag` and `pytensor.sparse.block_diag`. This function will be removed in a future release.", | |
| ) |
|
Thanks for looking into the issue @ricardoV94. |
pymc/math.py
Outdated
| def block_diagonal(matrices, sparse=False, format="csr"): | ||
| r"""See scipy.sparse.block_diag or | ||
| scipy.linalg.block_diag for reference | ||
| def block_diagonal(*matrices, sparse=False, format="csr"): |
There was a problem hiding this comment.
We should keep the old signature, missed this before:
| def block_diagonal(*matrices, sparse=False, format="csr"): | |
| def block_diagonal(matrices, sparse=False, format="csr"): |
|
Thanks @AryanNanda17 |
ricardoV94
changed the title
ricardoV94
changed the title
block_diag from math module in favor of PyTensor
The new code uses the sparse parameter to determine whether to call the sparse block_diag function (pytensor.sparse.basic.block_diag) or the dense version (pt.slinalg.block_diag).
Related Issue
block_diagfrompymc.mathin favor of alias topytensor.tensor.slinalg.block_diag#7085Checklist
Type of change
📚 Documentation preview 📚: https://pymc--7132.org.readthedocs.build/en/7132/