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

Propose the fix for #522. #523

Conversation

kellertuer
Copy link
Member

No description provided.

@kellertuer kellertuer added the Ready-for-Review A label for pull requests that are feature-ready label Aug 12, 2022
@kellertuer
Copy link
Member Author

Maybe an alternative to have both variants as a possibility would be to keep the function as is and do the Lie algebra projection for the group manifold analoga.

@kellertuer
Copy link
Member Author

I feel this variant should actually be the best choice: To have both.

For _Manigolds (Rotations, OrthogonalMatrices, UnitaryMatrices) we keep the representation as is (that is X is the tangent vector and not necessarily skew, but a rotated skew symmetric), for Lie groups (Orthogonal, SpecialOrthogonal, SpecialUnitary, Unitary) we use the Lie group representation. This way it is easy to have either of the representations, depending on what the user wants – in a precisely distinct way.

src/groups/general_unitary_groups.jl Outdated Show resolved Hide resolved
src/manifolds/GeneralUnitaryMatrices.jl Outdated Show resolved Hide resolved
@kellertuer
Copy link
Member Author

hm, seems really more complicated than I thought. So I will wait for the decision whether we want to unify it and in which way.

@kellertuer kellertuer removed the Ready-for-Review A label for pull requests that are feature-ready label Aug 13, 2022
@kellertuer
Copy link
Member Author

So if I understand correctly it is not only complicated to finish this but even more to get it work together with differentiation again? I would maybe propose to close this?

@mateuszbaran
Copy link
Member

This would be a lot of work to finish for not much benefit so I'd suggest closing it.

@kellertuer kellertuer closed this Aug 14, 2022
@kellertuer kellertuer deleted the kellertuer/change-TangentSpace-Repr-for-GeneralUnitary branch October 3, 2022 06:37
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.

2 participants