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

Rotation calculated for spherical particles #274

Merged
merged 5 commits into from
Nov 10, 2023

Conversation

rubenhorn
Copy link
Contributor

Description

Optimize rotation of zero vectors (spherical molecule simulations).
This case is handled explicitly to reduce the number of multiplications that are performed.
$\forall q\in\mathbb{H}:rotate(q,\vec{\scriptstyle 0})=\vec{\scriptstyle 0}$

For Quaternion::differentiate all summands in all equations have a factor from the input vector.
Thus, $\forall q\in\mathbb{H}:differentiate(q,\vec{\scriptstyle 0})=\vec{\scriptstyle 0}$

Resolved Issues

How Has This Been Tested?

As described in #267, the Argon and CO2 simulations were run using both the original and modified codes generating .xyz-files.
There was no difference between the .xyz-files which indicates that output of the computations is unchanged.

@rubenhorn rubenhorn linked an issue Sep 18, 2023 that may be closed by this pull request
@rubenhorn
Copy link
Contributor Author

Other optimizations may also be implemented 👉 Do NOT merge yet!

@rubenhorn rubenhorn changed the title Optimization of Quaternion for vector {0,0,0} Rotation calculated for spherical particles Sep 18, 2023
@rubenhorn
Copy link
Contributor Author

In a separate, very naive sequential test using C++/gcc, avoiding 100M calls to sqrt(1.0) for saves 10ms.
This is not much, but it may be notable when running very large simulations (particle count/time steps) without AutoPas.

@rubenhorn rubenhorn marked this pull request as ready for review November 7, 2023 12:55
@rubenhorn rubenhorn requested a review from HomesGH November 7, 2023 13:02
Copy link
Contributor

@HomesGH HomesGH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, it looks good besides a few smaller issues.
You could also have a look at a simulation with spherical particles using a profiler, e.g. valgrind, to identify the Quaternion functions with the highest impact.

src/molecules/Quaternion.cpp Outdated Show resolved Hide resolved
src/molecules/Quaternion.cpp Outdated Show resolved Hide resolved
src/molecules/Quaternion.h Outdated Show resolved Hide resolved
HomesGH
HomesGH previously approved these changes Nov 7, 2023
@rubenhorn
Copy link
Contributor Author

Using the GNU profiler (gprof) for the argon example (config_noAP_ALL.xml with <steps>10000</steps>, no output):

Method baseline (103.45s) optimized (95.71s)
Quaternion::rotate 0.56s (0.79%) 0.19s (0.30%)
Quaternion::rotateinv 0.21s (0.30%) 0.08s (0.12%)
Quaternion::differentiate 0.06s (0.09%) 0.10s (0.16%)

Reverting the change of Quaternion::differentiate .

@rubenhorn rubenhorn requested review from FG-TUM and HomesGH November 8, 2023 19:37
HomesGH
HomesGH previously approved these changes Nov 9, 2023
Copy link
Member

@FG-TUM FG-TUM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some small cmake stuff

cmake/modules/ALL.cmake Outdated Show resolved Hide resolved
Copy link
Member

@FG-TUM FG-TUM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@rubenhorn rubenhorn merged commit c28808b into master Nov 10, 2023
52 checks passed
@rubenhorn rubenhorn deleted the 267-rotation-calculated-for-spherical-particles branch November 10, 2023 12:21
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

Successfully merging this pull request may close these issues.

Rotation calculated for spherical particles
3 participants