Skip to content
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

Add support for decomposition complex tensors, to calculate the pseudo-inverse and the conjugate of a complex tensor #594

Merged
merged 5 commits into from
Oct 7, 2023

Conversation

AngelEzquerra
Copy link
Contributor

A lot of the code on this PR was actually written by @Vindaar, who provided the code to support the decomposition of complex tensors, to which I added a test case. In addition to that I have added pinv functions to calculate the pseudo-inverse (which are based on the svd decomposition functions) and made some of the common error functions support complex tensors.

Note that I made the pinv API match the corresponding numpy.linalg.pinv function API and in fact I copied most of the description from the numpy docs.

These changes were actually made by Vindaar <batsi90@gmail.com>. I only completed the SVG test.
…ix (pinv) and the conjugate of a complex tensor
Copy link
Collaborator

@Vindaar Vindaar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outside of the CI failure (requiring a address helper that uses unsafeAddr for older Nim) this is great! The result changes are not strictly needed of course.

src/arraymancer/linear_algebra/algebra.nim Outdated Show resolved Hide resolved
src/arraymancer/linear_algebra/algebra.nim Outdated Show resolved Hide resolved
Code to fix the issue was provided by @Vindaar.
@AngelEzquerra
Copy link
Contributor Author

Outside of the CI failure (requiring a address helper that uses unsafeAddr for older Nim) this is great! The result changes are not strictly needed of course.

I've fixed the CI failure on the latest commit as you suggested.
Regarding the explicit result assignment, I did not do it because I saw other procedures in the library that didn't do it either and because recently Araq adviced me to use this style for short procedures. However, if you think it is best to change it I will.

Co-authored-by: Vindaar <basti90@gmail.com>
@Vindaar Vindaar merged commit 1afd1c3 into mratsim:master Oct 7, 2023
6 checks passed
@Vindaar
Copy link
Collaborator

Vindaar commented Oct 7, 2023

Thanks a lot! 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants