-
Notifications
You must be signed in to change notification settings - Fork 191
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 SettleToConstantQuaternion #5703
Add SettleToConstantQuaternion #5703
Conversation
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.
Looks good. Please also fix the clang-tidy and compilation errors as well.
tests/Unit/Domain/FunctionsOfTime/Test_SettleToConstantQuaternion.cpp
Outdated
Show resolved
Hide resolved
tests/Unit/Domain/FunctionsOfTime/Test_SettleToConstantQuaternion.cpp
Outdated
Show resolved
Hide resolved
tests/Unit/Domain/FunctionsOfTime/Test_SettleToConstantQuaternion.cpp
Outdated
Show resolved
Hide resolved
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.
Looks good, you can squash. (you can optionally wait for #5706 to be merged and rebase so the tests pass)
493bd16
to
72cf009
Compare
Thanks again for the review, @knelli2 ! I squashed. A GrMhd test failure on gcc-9 release is unrelated to this PR (looks like tolerances might be too strict?) |
Test failures unrealted |
Proposed changes
Add new function of time like SettleToConstant but for quaternions; the function of time always returned a unit quaternion (i.e., a rotation).
Upgrade instructions
Code review checklist
make doc
to generate the documentation locally intoBUILD_DIR/docs/html
.Then open
index.html
.code review guide.
bugfix
ornew feature
if appropriate.Further comments