-
Notifications
You must be signed in to change notification settings - Fork 49
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
Properly Handle complex and float dtypes during directsum #57
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Daniel Ordonez <[email protected]>
@@ -562,11 +562,16 @@ def directsum(reprs: List[escnn.group.Representation], | |||
irreps += r.irreps | |||
|
|||
size = sum([r.size for r in reprs]) | |||
|
|||
cob = np.zeros((size, size)) |
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.
This is by default constructing np.float
matrices.
cob_inv = np.zeros((size, size)) | ||
|
||
# Determine the dtype for the change of basis diagonal matrix to avoid unsafe casting. | ||
dtype = np.complex if np.any([np.iscomplexobj(rep.change_of_basis) for rep in reprs]) else np.float |
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.
This seems a reasonable approach to handle dtypes
assuming user configures default numpy float and complex precisions.
Another approach is to dynamically check the safe nature of a cast, and in the case of unsafe casting update the type of cob
.
PS: What cob
stands for?
Hi @Danfoa The library does not support complex valued representations and, therefore, it is intentionally casting the matrices to float. Let me point you to the discussion here about complex representations. Let me know if that would solve your problem Gabriele |
Hi @Gabri95, I've been a bit tight with some deadlines so I have not answered our previous discussions, but know that I will. I already have the method for "realifying" representations, and for allowing to define representations from a mapping of group elements to unitary matrices. I do the Isotypic decomposition to find which complex irreps are present and then link those complex irreps to the real irreps of the group defined in ESCNN. I will make a PR soon for you to see this methods and evaluate if they are valuable to be added and how to better do introduced them in your API. I'll update/answer these comments as soon as I have some free time. Thanks for the support. |
Hi @Danfoa Sounds good, don't worry! Thanks for your contributions! 😄 Gabriele |
Hi @Gabri95, So I have some time now to answer, sorry for the delay. Let me give you a summary of what I implemented, and then we can discuss how to better contribute this to ESCNN. For my application (discrete symmetries of dynamical systems) we know from physics the group representation (usually unitary/orthogonal) acting on the system state configuration (e.g., generalized coordinates). Thus I needed to provide the representations of the group generator and "fill" the entire group representation for all "non-generator" actions. I do that with this function for a limited number of groups (for now): The function simply takes the generators of the group and their representation matrices (from physics) and gives the full group map from elements to unitary matrices. Now we need to actually instanciate a ESCNN representation. To do this we use the function: This function does the "real" isotypic decomposition (i.e., decomposition of a rep into complex/real irreps). This entire module is a bit engaged, although probably there is a lot of room for improvement. Hopefully is not to messy for you to follow what is going on. Basically I:
Then we have the change of basis and the list of real ESCNN irreps present in the original mapping, so we build the instance of the ESCNN representation. I have tested this for Cyclic, Dihedral and DirectProductGroup of these groups. Even for higher dimensional scenarios the Isotypic decomposition does not take much. With this script, I was able to generate all the representations for the robots in my library MorphoSymm Please let me know what you think. What should I improve to contribute it to the ESCNN (if it is useful) and how to structure it for contribution? Cheers, and again thank you for the library, it allows me to scale up while building MorphoSymm |
Hi @Danfoa Nice, this is really impressive! 😄
I am happy our library was helpful! This seems like a really cool project! Your code seems really useful, it would be nice to include it in our library!
Is this correct? Actually, our library already implements a couple of things which could be useful for (part of) this process. First of all, all finite groups have a Second, I've already implemented a function to construct a (real) irrep from the matrices evaluated at the generators of a group here. This function is used for example to construct some irreps of the Icosahedral group. Moreover, IrreducibleRepresentations can only be real but carry quite some information about the corresponding complex irreps which could be useful to convert your complex irreps into real ones.
But there is more! Depending on the type, these irreps are expressed in a particularly convenient basis such that it is very easy to "extract" their complex counterpart. This is described in Appendix C.3 of our paper. Our code assumes this basis and I could implement a method for IrreducibleRepresentation which evaluates to the corresponding complex irrep (up to conjugation). Could this be useful? Finally, this function can be used to numerically decompose a real representation into real irreps. Maybe the library could be adapted to support a process like
Do you think this process would be suitable for your use case too? Let me know! Best, |
By default, the
directsum
method is generatingnp.float
change_of_basis
matrices, regardless whether the subrepresentations of the dtype of the subrepresentationschange_of_basis
.If any subrepresentation has
np.complex
change of basis matrices, these are getting unsafely casted to float, which later triggers an assertion error on the non-orthogonality of the change of basis.