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

Compatibility of transformations code with Automatic Differentiation tools (Zygote) #89

Open
srikumarks opened this issue Jan 21, 2023 · 2 comments

Comments

@srikumarks
Copy link
Contributor

Zygote is the most used Automatic Differentiation tool and it doesn't support static arrays due to their use for mutations 1. DCM and Quaternion structures used in frame conversion code, which I'm trying to use with Zygote, use SMatrix and SVector for efficiency reasons as stated in the code comments. However that makes them incompatible with Zygote.

Having the transformation code be AD compatible is perhaps useful (broadly true for most libraries) and so posting this issue in that spirit. One way to address this in the library is to perhaps also support a third rotation representation that uses ordinary vectors/matrices (could be DCM based representation, but just uses a vector of 9 numbers or an ordinary Matrix instead of an SMatrix{3,3}).

This impacts the angle_to_rot functions defined in the ReferenceFrameRotations package - ReferenceFrameRotations/uTarN/src/conversions/angle_to_rot.jl. It looks at the moment that having angle_to_rot support the ordinary matrix as a return type should suffice to get all the r_eci_to_eci and r_eci_to_ecef and such functions to also support them when overloaded with the first argument being the type Matrix. Will also have to check whether the "multiply with vector to transform it" steps will also work properly.

(I might implement this, but posting an issue in advance for any comments or tips that could help with it ... with the best case being "it already exists"!)

Footnotes

  1. Apparently. I'm not conversant with all the details of why.

@ronisbr
Copy link
Member

ronisbr commented Jan 21, 2023

Hi @srikumarks !

Thanks for the information! I will check what can we do. The incompatibility with Zygote is caused because DCM and Quaternion are immutable? What is the source of the problem?

@srikumarks
Copy link
Contributor Author

The incompatibility with Zygote is caused because DCM and Quaternion are immutable? What is the source of the problem?

Not because they're immutable (being immutable is great), but because they use static arrays - SMatrix and SVector as the underlying representation. These don't get adjoints in Zygote and I haven't had much luck rolling one on my own too. Nothing I try in terms of a custom adjoint seems to work. I referred to this thread - FluxML/Zygote.jl#570 .

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

No branches or pull requests

2 participants