New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Matrix summation improvements #17241
Conversation
✅ Hi, I am the SymPy bot (v147). I'm here to help you write a release notes entry. Please read the guide on how to write release notes. Your release notes are in good order. Here is what the release notes will look like:
This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.5. Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it. Click here to see the pull request description that was parsed.
Update The release notes on the wiki have been updated. |
This looks good to me. |
Looks good! Can you try #16989 and add those tests that work? (And if both works, add a fixes to this?) |
I've only added the first case because the second one hangs in jordan form computation. |
Codecov Report
@@ Coverage Diff @@
## master #17241 +/- ##
=============================================
+ Coverage 74.549% 74.564% +0.014%
=============================================
Files 623 623
Lines 161739 161784 +45
Branches 37965 37966 +1
=============================================
+ Hits 120576 120633 +57
+ Misses 35817 35800 -17
- Partials 5346 5351 +5 |
References to other Issues or PRs
Brief description of what is fixed or changed
There are some issues with
sum
involving matrices, which are fixed here.Matrix was casted into MutableDenseMatrix after summation, but now, it will be casted into
ImmutableDenseMatrix
andImmutableSparseMatrix
respecting the original matrix class.I made the behavior not to retain the mutability though, because
sympify
would already cast mutable matrix into a immutable one.There was inefficient casting of sparse matrices into dense matrices in the original code.
And the speed had improved as
_mat
attributeAttributeError: 'Identity' object has no attribute '_mat'
But after now, it may not be computed because of lack of some formulas, but at least it will return unevaluated.
I've also changed the check like
because of the recursion error, if it does not succeed rewriting.
Other comments
Release Notes
Sum
casting dense matrix into sparse matrix.Sum
will cast mutable matrices into immutable variant after computation