-
Notifications
You must be signed in to change notification settings - Fork 2
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
FCHL19 with periodic boundary conditions and revised OpenMP parallelization #6
base: main
Are you sure you want to change the base?
Conversation
can you expand on this? Did you have to remove the compiler flags or notice that it wasn't allocating all the resources? |
To compile with OpenMP I edited several lines in
After compiling the code I compared its performance on a test problem to performance of the |
Verification script fixed and made more thorough.
Found a mistake in the way gradients for PBC were calculated and verified. |
qmllib is finally updated to work with numpy2.0 and openmp flags are fixed.
|
I successfully pulled from main and replaced references to |
tests/verification_fchl19_pbc.py
Outdated
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.
Can you rename the test file to start with test_
? It looks like the file was skipped during the test run.
I renamed the script and also changed it in order its CPU time was more in line with the other test scripts. TBH I am still not happy about the test’s CPU time consumption, but I do not see a way to make it better without augmenting |
Adds support for periodic boundary conditions when generating FCHL19 representations. Adds the verification script for checking PBC works correctly (temporarily placed into
tests
).facsf.f90
revised to avoid using REDUCTION construct in FCHL19's OpenMP parallelization while minimizing the additional CPU and memory usage compared to a serial implementation. Admittedly, I have doubts about how necessary including "!$OMP CRITICAL" was, but I cannot see a better alternative.Unfortunately, I had trouble getting
qmllib
to work with OpenMP for some reason, hence correctness offascf.f90
's parallelization was verified by using the same file in my fork of originalqml
.