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

[Lang] Add ti.eig for 2x2 matrices #2303

Merged
merged 6 commits into from
May 2, 2021
Merged

[Lang] Add ti.eig for 2x2 matrices #2303

merged 6 commits into from
May 2, 2021

Conversation

Robslhc
Copy link
Contributor

@Robslhc Robslhc commented Apr 29, 2021

Related issue = #2208, #2293

[Click here for the format server]


This PR aims to implement utility functions ti.eig() for 2x2 matrices as described in #2293.

Comment on lines +107 to +119
else:
lambda1 = ti.Vector([tr, ti.sqrt(-gap)]) * 0.5
lambda2 = ti.Vector([tr, -ti.sqrt(-gap)]) * 0.5
A1r = A - lambda1[0] * ti.Matrix.identity(dt, 2)
A1i = -lambda1[1] * ti.Matrix.identity(dt, 2)
A2r = A - lambda2[0] * ti.Matrix.identity(dt, 2)
A2i = -lambda2[1] * ti.Matrix.identity(dt, 2)
v1 = ti.Vector([A2r[0, 0], A2i[0, 0], A2r[1, 0],
A2i[1, 0]]).cast(dt).normalized()
v2 = ti.Vector([A1r[0, 0], A1i[0, 0], A1r[1, 0],
A1i[1, 0]]).cast(dt).normalized()
eigenvalues = ti.Matrix.rows([lambda1, lambda2])
eigenvectors = ti.Matrix.cols([v1, v2])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there is one thing that needs to be discussed. From my knowledge, taichi doesn't have such a datatype to represent complex numbers, which could be generated by eigensolvers.

Currently, I use 2 numbers to store the real part and imaginary part of a complex number. That is to say, for a 2x2 matrix, function ti.eig() returns a 2x2 matrix v and a 4x2 matrix w. Each row of matrix v stores one eigenvalue represented by a 2d vector for its real and imaginary part. Each column of w stores one eigenvector with 2 entries each represented by two numbers for its real and imaginary part. I'm not sure whether this is the best way to handle this problem. Is there any advice on this? @k-ye

Copy link
Member

Choose a reason for hiding this comment

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

I see. Yeah this sounds good, thanks! Could you write this as the docstring?

Copy link
Member

@k-ye k-ye left a comment

Choose a reason for hiding this comment

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

Thank you! LGTM + nits

if dt is None:
dt = impl.get_runtime().default_fp
from .linalg import eig
return eig(A, dt)
Copy link
Member

Choose a reason for hiding this comment

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

While this style is probably following how svd() is implemented, it could be a bit confusing to have multiple eig() symbols. I'd suggest to change to this:

def eig(A, dt=None):
    if dt is None:
        dt = impl.get_runtime().default_fp
    from taichi.lang import linalg
    if A.n == 2:
      return linalg.eig2x2(A, dt)
    raise Exception("Eigen solver only supports 2D matrices.")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. I will change to this and commit my implementation of sym_eig3x3() together.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I think it's better to split into two PRs. This makes the review process easier ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I'll fix it.

Comment on lines +107 to +119
else:
lambda1 = ti.Vector([tr, ti.sqrt(-gap)]) * 0.5
lambda2 = ti.Vector([tr, -ti.sqrt(-gap)]) * 0.5
A1r = A - lambda1[0] * ti.Matrix.identity(dt, 2)
A1i = -lambda1[1] * ti.Matrix.identity(dt, 2)
A2r = A - lambda2[0] * ti.Matrix.identity(dt, 2)
A2i = -lambda2[1] * ti.Matrix.identity(dt, 2)
v1 = ti.Vector([A2r[0, 0], A2i[0, 0], A2r[1, 0],
A2i[1, 0]]).cast(dt).normalized()
v2 = ti.Vector([A1r[0, 0], A1i[0, 0], A1r[1, 0],
A1i[1, 0]]).cast(dt).normalized()
eigenvalues = ti.Matrix.rows([lambda1, lambda2])
eigenvectors = ti.Matrix.cols([v1, v2])
Copy link
Member

Choose a reason for hiding this comment

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

I see. Yeah this sounds good, thanks! Could you write this as the docstring?

@ljcc0930 ljcc0930 requested a review from k-ye April 30, 2021 03:13
@Robslhc Robslhc changed the title [Lang] Add ti.eig for 2x2 matrices and symmetric 3x3 matrices [Lang] Add ti.eig for 2x2 matrices Apr 30, 2021
@Robslhc Robslhc marked this pull request as ready for review April 30, 2021 07:00
@Robslhc
Copy link
Contributor Author

Robslhc commented Apr 30, 2021

The CI of GPU failed but I have no idea about the error. It seems the error is not related to my changes, is it?

@k-ye
Copy link
Member

k-ye commented Apr 30, 2021

No, it's an issue on the CI bot itself #2307 ..

@k-ye k-ye merged commit abd063c into taichi-dev:master May 2, 2021
@BillXu2000 BillXu2000 mentioned this pull request May 13, 2021
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