-
Notifications
You must be signed in to change notification settings - Fork 6
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
Cleanup Special Matrices code #202
Cleanup Special Matrices code #202
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, this is ready to merge once the tests pass. I have added some unit tests for the special matrices.
replace comm_matrix helper functions with a CommutationMatrix and overloaded linalg ops
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## devel #202 +/- ##
==========================================
+ Coverage 67.86% 69.54% +1.68%
==========================================
Files 51 52 +1
Lines 2483 2423 -60
==========================================
Hits 1685 1685
+ Misses 798 738 -60 ☔ View full report in Codecov by Sentry. |
Thanks for the additional tests. Should we merge now? It seems like formatter still complains... |
It looks ready to me. |
cdc4415
into
StructuralEquationModels:devel
Thank you for reviewing and merging! I'm preparing a next portion of commits to cherry-pick, but it would have to wait a day or two. |
Simplifies a bit the code to generate elimination, duplication and commutation matrices.
The latter is implemented as a special
CommutationMatrix <: AbstractMatrix{Int}
subtype, which provides elegant integration into LinearAlgebra API.I was actually trying to check, whether there are Julia packages implementing these special matrices.
So far I have found VectorTransformations.jl package, but it works by constructing sparse matrices with the predefined structure, not by implementing custom code that does the expected transformation directly (which might be faster).
Cherry-picked from #193