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

Optimize2 #358

Merged
merged 110 commits into from
Apr 28, 2023
Merged

Optimize2 #358

merged 110 commits into from
Apr 28, 2023

Conversation

lbluque
Copy link
Collaborator

@lbluque lbluque commented Apr 19, 2023

Summary

  • Enables openmp based parallelism for computing correlation vectors, cluster interactions, and their changes from single flips.

  • Partially addresses Improve monte-carlo speed further by reducing python overheads #327
    Speed-ups can be substantial, up to 4x in the systems I tested. However with this speed-up the bottleneck in most MC will now be in the remaining python code that is run at each step (especially MCUsher.propose_step)

  • This required a re-writing of the cython functions into cython extension classes, and implementing C-struct containers to allow accessing the correlation/interaction tensors that are saved as tuples of ndarrays of different dimensions in Python.

  • Moved packaging to use pyproject.toml + setup.py

Additional dependencies introduced (if any)

  • OpenMP is needed for parallelism (smol should still build in sequential mode if openmp is not available).

TODO

  • Building wheels for macos in CI since C complilers do not have openmp as default.
  • Add how to build smol from source with openmp in the Developing section of the documentation.
  • Include an example notebook setting the different numbers of threads for calculations and plot resulting speed-ups.

Checklist

@lbluque lbluque marked this pull request as ready for review April 26, 2023 21:39
@lbluque lbluque changed the title [WIP] Optimize2 Optimize2 Apr 26, 2023
@lbluque
Copy link
Collaborator Author

lbluque commented Apr 26, 2023

I noticed an issue with correctly checking openmp support on Debian Linux, where the check_openmp_support function used to install returns False, but forcing the install by skipping the check successfully builds with openmp support.

I do not think it is critical but will investigate further.

@lbluque
Copy link
Collaborator Author

lbluque commented Apr 27, 2023

I noticed an issue with correctly checking openmp support on Debian Linux, where the check_openmp_support function used to install returns False, but forcing the install by skipping the check successfully builds with openmp support.

I do not think it is critical but will investigate further.

The problem comes from using the system gcc and openmp libraries with conda's linker. Since that is not an issue with our code, I think we can proceed to consider merging this.

Copy link
Collaborator

@qchempku2017 qchempku2017 left a comment

Choose a reason for hiding this comment

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

These are a great amount of excellent work! Thank you so much for doing this to the group!

@lbluque lbluque merged commit 3018d54 into CederGroupHub:main Apr 28, 2023
@lbluque lbluque deleted the optimize2 branch April 28, 2023 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants