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

Decentralizing QE #184

Merged
merged 49 commits into from
Sep 29, 2022
Merged

Decentralizing QE #184

merged 49 commits into from
Sep 29, 2022

Conversation

elinscott
Copy link
Collaborator

@elinscott elinscott commented Sep 20, 2022

This PR decentralizes the way we treat our installations of QE. Specifically:

  • quantum_espresso/kcp and quantum_espresso/utils are now separate repositories https://github.com/epfl-theos/koopmans-kcp and https://github.com/epfl-theos/koopmans-qe-utils, and are included in koopmans as submodules
  • the default behaviour when looking for an executable such as pw.x is now to look for pw.x (without any path specified) rather than looking in /path/to/koopmans/bin/ specifically
  • a make install procedure has been added that installs QE binaries on the user's path (/usr/local/bin/ by default)
  • we no longer have gh actions that test the compilation of fortran code (this is instead now performed in the repo corresponding to the submodule in question)

It also includes various changes making the project more amenable to python -m build:

  • moved over to installing via pyproject.toml in place of setup.py + various .ini files
  • koopmans has been moved to koopmans/src

@elinscott elinscott marked this pull request as draft September 20, 2022 09:19
@codecov-commenter
Copy link

codecov-commenter commented Sep 20, 2022

Codecov Report

Base: 83.15% // Head: 83.31% // Increases project coverage by +0.16% 🎉

Coverage data is based on head (1cc75ce) compared to base (b8e2104).
Patch coverage: 89.18% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #184      +/-   ##
==========================================
+ Coverage   83.15%   83.31%   +0.16%     
==========================================
  Files          66       65       -1     
  Lines        5573     5562      -11     
==========================================
  Hits         4634     4634              
+ Misses        939      928      -11     
Impacted Files Coverage Δ
koopmans/calculators/__init__.py 100.00% <ø> (ø)
koopmans/workflows/_folding.py 53.33% <0.00%> (ø)
koopmans/calculators/_utils.py 78.32% <75.00%> (+0.38%) ⬆️
koopmans/utils/_os.py 90.90% <80.00%> (+16.66%) ⬆️
koopmans/calculators/_koopmans_cp.py 88.18% <100.00%> (ø)
koopmans/calculators/_koopmans_ham.py 55.73% <100.00%> (ø)
koopmans/calculators/_koopmans_screen.py 92.00% <100.00%> (ø)
koopmans/calculators/_ph.py 90.00% <100.00%> (ø)
koopmans/calculators/_projwfc.py 87.32% <100.00%> (ø)
koopmans/calculators/_pw.py 64.15% <100.00%> (ø)
... and 9 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@elinscott elinscott marked this pull request as ready for review September 23, 2022 10:49
@elinscott elinscott requested a review from degennar September 23, 2022 10:49
@degennar
Copy link
Collaborator

Just to understand, why do we have the executables in /usr/local/bin/ (which requires sudo rights)?

@elinscott
Copy link
Collaborator Author

elinscott commented Sep 27, 2022

Just to understand, why do we have the executables in /usr/local/bin/ (which requires sudo rights)?

This is just the default behaviour, following the default behaviour of QE. Of course a user can install the binaries wherever they like on their system as long as they are on their path; this can be achieved with make install PREFIX=/path/to/bin/.

I suppose the point is here make install adds the binaries to a location that is already on PATH, rather than the outdated approach of adding the location of the binaries to PATH.

@elinscott elinscott linked an issue Sep 27, 2022 that may be closed by this pull request
@elinscott elinscott self-assigned this Sep 28, 2022
@elinscott elinscott merged commit b009adb into master Sep 29, 2022
@elinscott elinscott deleted the decentralizing_qe branch September 29, 2022 12:23
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.

Replace soon-to-be-deprecated distutils
3 participants