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

FCHL19 with periodic boundary conditions and revised OpenMP parallelization #6

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

kvkarandashev
Copy link

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 of fascf.f90's parallelization was verified by using the same file in my fork of original qml.

@charnley charnley requested a review from andersx April 1, 2024 16:02
@charnley
Copy link
Member

charnley commented Apr 1, 2024

Unfortunately, I had trouble getting qmllib to work with OpenMP for some reason

can you expand on this? Did you have to remove the compiler flags or notice that it wasn't allocating all the resources?

@kvkarandashev
Copy link
Author

To compile with OpenMP I edited several lines in _compile.py as follows:

COMPILER_FLAGS = ["-O3", "-fopenmp", "-m64", "-march=native", "-fPIC",
                     "-Wno-maybe-uninitialized", "-Wno-unused-function", "-Wno-cpp"]
extra_flags = ["-lgomp", "-lpthread", "-lm", "-ldl"] + COMPILER_FLAGS

flags = ["-L/usr/lib/", "-lblas", "-llapack"] + extra_flags

After compiling the code I compared its performance on a test problem to performance of the qml fork with the same fascf.f90 file. qmllib never seemed to use more than one core, as supported by the timings:
OMP_NUM_THREADS=1:
qml: 0.7533 mins
qmllib: 0.8178 mins
OMP_NUM_THREADS=20:
qml: 0.1390 mins
qmllib: 0.8145 mins

@charnley charnley mentioned this pull request Apr 1, 2024
Verification script fixed and made more thorough.
@kvkarandashev
Copy link
Author

Found a mistake in the way gradients for PBC were calculated and verified.
Fixed the verification script and added verification via finite difference.

@charnley
Copy link
Member

charnley commented Nov 8, 2024

qmllib is finally updated to work with numpy2.0 and openmp flags are fixed.

  • Can you pull from main?
  • Use pathlib.Path instead of os and glob

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.

2 participants