-
Notifications
You must be signed in to change notification settings - Fork 56
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 check_point
for complex SymplecticStiefel
#735
Comments
Can you maybe provide an example? I just did
If I assume that |
Does it actually return
I noticed it when checking this ambiguity: check_point(M::SymplecticStiefel{<:Any, ℝ}, p; kwargs...) @ Manifolds ~/.julia/dev/Manifolds/src/manifolds/SymplecticStiefel.jl:113
check_point(M::SymplecticStiefel, p::StiefelPoint; kwargs...) @ Manifolds ~/.julia/dev/ManifoldsBase/src/point_vector_fallbacks.jl:119 |
right, that I did not check, I thought it was failing somwhere. You are right then it probably just checks the embedding, i.e. field type and dimensions for now. but p^+p just has to be checked on complex as well then. I can check whether I see how much to do that is (maybe even just removing R) |
Hm, I checked a bit the code. First of all, the complex part (also of p above) is zero, so I am not sure we can generate (non real) symplectic matrices. So I am also not sure what to actually additionally check. To conclude, I would consider this not fully installed, but also not much I can do here (neither good-first nor easy to extend) since I am not aware of Literature that could help us here. edit: One could make the corresponding doc strings a bit more precise, that for those we do not know yet how that has to work in the complex case. That improbably also why it was not implemented in the first place. |
It seems |
I see, I didn't really check how difficult that would be to add because it looked quite simple but maybe it isn't actually. |
I think the problem is that the main idea of the model here is to cover real-valued things; maybe I was too optimistic adding the complex case to the manifold at all. For example the paper is also only called the real-valued symplectic Stiefel. |
Baby the best is then to remove the complex case (we have a breaking PR open anyways) also for symplectic Grassmann? I could not find any literature on the complex case |
I think it would be enough to limit signatures of methods that don't work in the complex case. |
But that is the current case already. Then the complex case falls back to our optimistic check-default that returns nothing. |
Ah, right, then maybe let's indeed limit symplectic Stiefel and Grassmann to real numbers for now. |
Great. On the 0.10 PR? Could you add that or shall I? |
I will do that on the 0.10 PR. |
The existing one only works for real numbers.
The text was updated successfully, but these errors were encountered: