Implemented direct conversion to/from euler angles in any sequence #24358

evbernardes · 2022-12-08T21:09:50Z

Brief description of what is fixed or changed

I recently published an article about a direct formula for the conversion between a quaternion variable to Euler angles in any sequence, which can be read here (Open Access).

Compared to either having 12 separate formulas (or 24, taking into account both intrinsic and extrinsic rotations) or using the well known quaternion-to-matrix-to-euler method (used for SciPy, for example), this has 3 main advantages:

Numerically, it is up to 30 times faster than the previous quaternion-to-matrix-to-euler method (used for SciPy, for example).
It is a lot simpler to implement, debug and maintain than both methods.
It provides the simple formulas that, I imagine, can be used for theorerical work, which is why I think it could be useful in Sympy

Other comments

The axis sequence is defined by a string of length 3, which can represent either extrinsic (when the string is all uppercase) or intrinsic rotations (when all lowercase). This choice was for consistency with the Rotation class in SciPy, since it could be beneficial for the whole Python scientific community to have this consistency.

I also implemented a test that compares transforms a quaternion into euler angles then back to a quaternion, to show it gives the same result. This is done for all possible 24 seq cases.

Since the publication, this method has been merged into the main branch of SciPy (see pull request here).

Release Notes

algebras
- Added direct conversion between Euler angles and quaternions.

smichr · 2022-12-09T05:52:52Z

sympy/algebras/quaternion.py

+    axes = set(['x', 'y', 'z'])
+    if not (i in axes and j in axes and k in axes):
+        raise ValueError("Expected axes from `seq` to be from "
+                         "['x', 'y', 'z'] or ['X', 'Y', 'Z'], "
+                         "got {}".format(seq))


Suggested change

axes = set(['x', 'y', 'z'])

if not (i in axes and j in axes and k in axes):

raise ValueError("Expected axes from `seq` to be from "

"['x', 'y', 'z'] or ['X', 'Y', 'Z'], "

"got {}".format(seq))

bad = set(seq) - set('xyzXYZ')

if bad:

raise ValueError("Expected axes from `seq` to be from "

"['x', 'y', 'z'] or ['X', 'Y', 'Z'], "

"got {}".format(''.join(bad)))

smichr · 2022-12-09T05:58:17Z

sympy/algebras/quaternion.py

+    if axis == 'x':
+        return [1, 0, 0]
+    if axis == 'y':
+        return [0, 1, 0]
+    if axis == 'z':
+        return [0, 0, 1]


Suggested change

if axis == 'x':

return [1, 0, 0]

if axis == 'y':

return [0, 1, 0]

if axis == 'z':

return [0, 0, 1]

return [1 if i == axis else 0 for i in 'xyz']

Nice one-liner! Wouldn't this be harder for others to read?

The simple form seems pretty clear to me.

The simple form seems pretty clear to me.

I changed the code and put the suggested one-liner instead of calling the functions. This I can avoid adding two more function definitions to the code :)

smichr · 2022-12-09T05:59:57Z

sympy/algebras/quaternion.py

+    if axis == 'x': return 1
+    if axis == 'y': return 2
+    if axis == 'z': return 3


Suggested change

if axis == 'x': return 1

if axis == 'y': return 2

if axis == 'z': return 3

return 'xyz'.index(axis) + 1

Same as for _elementary_axis! Since they are oneliners, I wonder if it isn't better to just put them on their calls instead of defining these functions.

I think I'm gonna do that instead, to lower the diff.

smichr · 2022-12-09T06:06:57Z

Looks interesting (though I do not work in this area).

I made a few minor suggestions and added a "release notes" comment which should satisfy the bot. Check the test results to see how some tests are expected to be formatted. You should be able to copy/paste the desired output into the SymPy file.

evbernardes · 2022-12-09T07:16:03Z

Looks interesting (though I do not work in this area).

Thanks! :)

I made a few minor suggestions and added a "release notes" comment which should satisfy the bot. Check the test results to see how some tests are expected to be formatted. You should be able to copy/paste the desired output into the SymPy file.

I integrated the suggestions and added a line to .mailmap, and fixed the doctest!

About the "release notes", where can I see it? Apparently the bot deleted the comment!

sympy-bot · 2022-12-09T07:42:31Z

✅

Hi, I am the SymPy bot (version not found!). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

algebras
- Added direct conversion between Euler angles and quaternions. (#24358 by @evbernardes)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.12.

Click here to see the pull request description that was parsed.

#### Brief description of what is fixed or changed
I recently published an article about a direct formula for the conversion between a quaternion variable to Euler angles in any sequence, which can be read [here](https://journals.plos.org/plosone/article?id=10.1371/journal.pone.0276302) (Open Access).

Compared to either having 12 separate formulas (or 24, taking into account both intrinsic and extrinsic rotations) or using the well known quaternion-to-matrix-to-euler method (used for SciPy, for example), this has 3 main advantages: 

1. Numerically, it is up to 30 times faster than the previous quaternion-to-matrix-to-euler method (used for SciPy, for example).
2. It is a lot simpler to implement, debug and maintain than both methods.
3. It provides the simple formulas that, I imagine, can be used for theorerical work, which is why I think it could be useful  in Sympy

#### Other comments

The axis sequence is defined by a string of length 3, which can represent either extrinsic (when the string is all uppercase) or intrinsic rotations (when all lowercase). This choice was for consistency with the `Rotation` class in SciPy, since it could be beneficial for the whole Python scientific community to have this consistency.

I also implemented a test that compares transforms a quaternion into euler angles then back to a quaternion, to show it gives the same result. This is done for all possible 24 seq cases.

Since the publication, this method has been merged into the main branch of SciPy [(see pull request here)](https://github.com/scipy/scipy/pull/17392).

#### Release Notes

<!-- BEGIN RELEASE NOTES -->
* algebras
  * Added direct conversion between Euler angles and quaternions.
<!-- END RELEASE NOTES -->

github-actions · 2022-12-09T09:03:55Z

Benchmark results from GitHub Actions

Lower numbers are good, higher numbers are bad. A ratio less than 1
means a speed up and greater than 1 means a slowdown. Green lines
beginning with + are slowdowns (the PR is slower then master or
master is slower than the previous release). Red lines beginning
with - are speedups.

Significantly changed benchmark results (PR vs master)

Significantly changed benchmark results (master vs previous release)

       before           after         ratio
     [41d90958]       [bd7bbeaa]
     <sympy-1.11.1^0>                 
-     1.17±0.03ms         774±20μs     0.66  solve.TimeSparseSystem.time_linear_eq_to_matrix(10)
-      3.65±0.3ms      1.45±0.05ms     0.40  solve.TimeSparseSystem.time_linear_eq_to_matrix(20)
-      6.91±0.1ms      2.18±0.06ms     0.32  solve.TimeSparseSystem.time_linear_eq_to_matrix(30)

Full benchmark results can be found as artifacts in GitHub Actions
(click on checks at the top of the PR).

evbernardes · 2022-12-09T09:05:34Z

Alright, all tests are passing now :) I'm just gonna make a last minor edit.

We can see that the benchmark from github actions shows the speed has changed, which doesn't surprised me because I implemented a test that symbolic shows that applying to_euler and then from_euler gives back the same result using simplify.

~~I think it's worth it to leave it like this.~~

EDIT.: Ok I just realized I misinterpreted the message from the actions bot. This is a speedup from the master.

sympy/algebras/quaternion.py

sympy/algebras/tests/test_quaternion.py

sympy/algebras/quaternion.py

smichr · 2022-12-09T20:41:47Z

ping @NikhilPappu

evbernardes · 2022-12-10T06:47:28Z

@smichr thanks for reviewing the code! I'll be modifying it next Monday :)

sympy/algebras/quaternion.py

@smichr

As suggested by @smichr Co-authored-by: Christopher Smith <smichr@gmail.com>

@smichr

As suggested by @smichr Co-authored-by: Christopher Smith <smichr@gmail.com>

@smichr

As suggested by @smichr Co-authored-by: Christopher Smith <smichr@gmail.com>

evbernardes · 2022-12-12T18:33:54Z

@smichr and @sylee957, I integrated your last suggestions!

smichr · 2022-12-12T22:56:52Z

sympy/algebras/quaternion.py

@@ -84,6 +112,158 @@ def d(self):
    def real_field(self):
        return self._real_field

+    @classmethod
+    def from_euler(cls, angles, seq):
+        """Returns quaternion equivalent to Euler angles represented same in


Suggested change

"""Returns quaternion equivalent to Euler angles represented same in

"""Returns quaternion equivalent to Euler angles represented in

Oops, that's indeed a typo! I meant to say Returns quaternion equivalent to rotation represented by the Euler angles.

smichr · 2022-12-12T22:58:49Z

sympy/algebras/quaternion.py

+            return trigsimp(qi * qj * qk)
+
+    def to_euler(self, seq):
+        """Returns Euler angles representing same in the sequence given by


same issue as above -- "representing same". I am not sure what is the same, i.e. it's unclear.

Same as last one, I meant "same rotation". Fixed both!

smichr · 2022-12-13T17:11:05Z

Thanks. This seems like a "sliced bread" commit -- a nice addition for those that use it.

evbernardes · 2022-12-13T17:29:55Z

Thanks. This seems like a "sliced bread" commit -- a nice addition for those that use it.

Awesome! Thank you for merging it :)

evbernardes added 2 commits December 8, 2022 21:54

implemented to_euler and from_euler

1d85764

minor edit

6fe28f6

evbernardes changed the title ~~Implemented methods for direct conversion to and from euler angles~~ Implemented direct conversion to/from euler angles in any sequence Dec 8, 2022

added reference

69baa8d

smichr reviewed Dec 9, 2022

View reviewed changes

sympy deleted a comment from sympy-bot Dec 9, 2022

evbernardes added 3 commits December 9, 2022 08:01

simplified basis vectors declaration

f1a568b

simplified _check_sequence

ca12931

added author

e9b9699

fixed doctest

fc95b44

minor edit

e0db748

smichr reviewed Dec 9, 2022

View reviewed changes

sympy/algebras/quaternion.py Outdated Show resolved Hide resolved

smichr reviewed Dec 9, 2022

View reviewed changes

sympy/algebras/quaternion.py Outdated Show resolved Hide resolved

smichr reviewed Dec 9, 2022

View reviewed changes

sympy/algebras/quaternion.py Outdated Show resolved Hide resolved

smichr reviewed Dec 9, 2022

View reviewed changes

sympy/algebras/quaternion.py Outdated Show resolved Hide resolved

smichr reviewed Dec 9, 2022

View reviewed changes

sympy/algebras/quaternion.py Outdated Show resolved Hide resolved

smichr reviewed Dec 9, 2022

View reviewed changes

sympy/algebras/tests/test_quaternion.py Outdated Show resolved Hide resolved

smichr reviewed Dec 9, 2022

View reviewed changes

sympy/algebras/quaternion.py Outdated Show resolved Hide resolved

sylee957 reviewed Dec 10, 2022

View reviewed changes

sympy/algebras/quaternion.py Outdated Show resolved Hide resolved

evbernardes and others added 2 commits December 12, 2022 18:30

Changed _check_sequence to _is_extrinsic

1823aa5

As suggested by @smichr Co-authored-by: Christopher Smith <smichr@gmail.com>

Checking type of seq before length

1ba01e2

As suggested by @smichr Co-authored-by: Christopher Smith <smichr@gmail.com>

evbernardes and others added 3 commits December 12, 2022 18:31

Correcting typos

77c6eac

As suggested by @smichr Co-authored-by: Christopher Smith <smichr@gmail.com>

simplified test_to_euler

81d0739

to_euler now returns tuple

a4afaff

updated .mailmap

5f806c4

smichr reviewed Dec 12, 2022

View reviewed changes

minor edit in doc

45ec0ac

smichr merged commit db1d91d into sympy:master Dec 13, 2022

This was referenced Dec 13, 2022

Implementation of conversion methods to / from euler angles in any sequence moble/quaternionic#40

Open

Added two option to Quaternion.to_euler #24402

Merged

Added direct quaternion to euler transformation and solved issue #53 matthew-brett/transforms3d#54

Open

This was referenced Dec 30, 2022

Implement direct conversions to and from quaternions and Euler angles ros2/geometry2#577

Open

Implement direct conversion between quaternions and euler angles in any sequence dfki-ric/pytransform3d#207

Closed

This was referenced Jan 11, 2023

Implement direct conversion between quaternions and euler angles in any sequence mrdoob/three.js#25275

Closed

Added general conversion functions between Quaternions and Euler angles mrdoob/three.js#25277

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented direct conversion to/from euler angles in any sequence #24358

Implemented direct conversion to/from euler angles in any sequence #24358

evbernardes commented Dec 8, 2022 •

edited by asmeurer

Loading

smichr Dec 9, 2022

smichr Dec 9, 2022 •

edited

Loading

evbernardes Dec 9, 2022

moorepants Dec 9, 2022

evbernardes Dec 9, 2022

smichr Dec 9, 2022

evbernardes Dec 9, 2022

smichr commented Dec 9, 2022

evbernardes commented Dec 9, 2022 •

edited

Loading

sympy-bot commented Dec 9, 2022 •

edited

Loading

github-actions bot commented Dec 9, 2022 •

edited

Loading

evbernardes commented Dec 9, 2022 •

edited

Loading

smichr commented Dec 9, 2022

evbernardes commented Dec 10, 2022

evbernardes commented Dec 12, 2022

smichr Dec 12, 2022

evbernardes Dec 13, 2022

smichr Dec 12, 2022

evbernardes Dec 13, 2022

smichr commented Dec 13, 2022

evbernardes commented Dec 13, 2022

	"""Returns quaternion equivalent to Euler angles represented same in
	"""Returns quaternion equivalent to Euler angles represented in

Implemented direct conversion to/from euler angles in any sequence #24358

Implemented direct conversion to/from euler angles in any sequence #24358

Conversation

evbernardes commented Dec 8, 2022 • edited by asmeurer Loading

Brief description of what is fixed or changed

Other comments

Release Notes

Choose a reason for hiding this comment

smichr Dec 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smichr commented Dec 9, 2022

evbernardes commented Dec 9, 2022 • edited Loading

sympy-bot commented Dec 9, 2022 • edited Loading

github-actions bot commented Dec 9, 2022 • edited Loading

evbernardes commented Dec 9, 2022 • edited Loading

smichr commented Dec 9, 2022

evbernardes commented Dec 10, 2022

evbernardes commented Dec 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smichr commented Dec 13, 2022

evbernardes commented Dec 13, 2022

evbernardes commented Dec 8, 2022 •

edited by asmeurer

Loading

smichr Dec 9, 2022 •

edited

Loading

evbernardes commented Dec 9, 2022 •

edited

Loading

sympy-bot commented Dec 9, 2022 •

edited

Loading

github-actions bot commented Dec 9, 2022 •

edited

Loading

evbernardes commented Dec 9, 2022 •

edited

Loading