-
Notifications
You must be signed in to change notification settings - Fork 161
REFACTOR: Refactored quaternion implementation #6151
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
Conversation
…is, axis_to_rotation_matrix
Thanks for opening a Pull Request. If you want to perform a review write a comment saying: @ansys-reviewer-bot review |
…e' into Fix_coordinate_system_mode_change
# Conflicts: # tests/system/general/test_02_3D_modeler.py
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6151 +/- ##
==========================================
- Coverage 85.23% 85.19% -0.04%
==========================================
Files 167 169 +2
Lines 63737 63991 +254
==========================================
+ Hits 54325 54518 +193
- Misses 9412 9473 +61 🚀 New features to boost your workflow:
|
@Alberto-DM I like a lot this PR. I have two requests, if I am not wrong these 2 new classes are not part of any section of the documentation, I think it would be great to add both. Maybe here: https://aedt.docs.pyansys.com/version/stable/API/Primitives3D.html or https://aedt.docs.pyansys.com/version/stable/API/generic.html And, I think it would be useful to add an example of in PyAEDT examples repository to show the power of this class (the quaternion one mainly), what do you think? Maybe here: https://examples.aedt.docs.pyansys.com/version/dev/examples/aedt_general/modeler/index.html |
@Samuelopez-ansys, quaternions are powerful, but here are "just" used to convert between different coordinate systems. They are also used to translate a CS that is referring to another CS, which is referring to a third CS and so on.... It makes easy to get the equivalent rotation respect to Global. |
I think you could do something similar to this https://raw.githubusercontent.com/ansys/pyaedt/refs/heads/main/doc/source/API/generic.rst But much simpleer, because just defining the class, it will automatically create the doc of all the methods of the class Like here the farfield: https://raw.githubusercontent.com/ansys/pyaedt/refs/heads/main/doc/source/API/visualization/advanced.rst |
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.
Everything looks correct to me. I've made just a couple of optional suggestions.
Achieving 100% coverage would be the icing on the cake ;)
@lorenzovecchietti 94.43% coverage is the max I can do... |
Description
The quaternion implementation used for the coordinate systems rotation was incomplete and unable to handle special cases.
Now the quaternions have their own module, named
Quaternion
with the relevant algebra and operations defined.As an added bonus, all possible Euler rotations are now supported, not just ZXZ and ZYZ.
A
MathUtils
module has been also created to contain some mathematical utility functions.Extensive unit tests have been also added.
Issue linked
Address the issue #6037