-
Notifications
You must be signed in to change notification settings - Fork 18
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
use python script to setup all codes #706
use python script to setup all codes #706
Conversation
Hi @AndresOrtegaGuerrero , you can test this by deleting some of the codes (not used yet by QEApp), and install them again.
|
|
||
|
||
def _generate_string_to_setup_code(code_name, computer_name="localhost"): | ||
"""Generate the python string to setup a code for a given computer.""" | ||
try: | ||
load_code(f"{code_name}-{QE_VERSION}@localhost") |
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.
Should it be load_code(f"{code_name}-{QE_VERSION}@{computer_name}")
. ?
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.
Yes, you are right.
|
||
|
||
def _generate_string_to_setup_code(code_name, computer_name="localhost"): | ||
"""Generate the python string to setup a code for a given computer.""" |
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.
Generate the python string to setup an AiiDA code for a given computer. Tries to load an existing code and if not existent, generates Python code to create and store a new code setup.
Should this PR be dependent on the |
This was my test Codes are setup! real 0m7.560s |
Hi @AndresOrtegaGuerrero , thanks for the review. I made the changes as you suggested.
It could be if the Python API of AiiDA changes, e.g., |
Thanks for the test. You can try to run this in the main branch, and check the time difference. |
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.
LGTM, my test show less time
It takes 1-2 sec to set up a code, and we have 14 codes (pw, dos, pp, ph, etc.), so around 20 sec in total. Which is not good, and it takes too long to load the container.
I looked into the issue and found that most of the time is used to set up the verdi command, profile and database. Thus it's would be good to combine all code set up as one.
Since the code setup is run in a different thread, thus, using the
InstalledCode
directly will fail when storing the code, as I tested in #695. This PR uses thepython -c
command to run in thesubprocess
.The total time to set up all 14 codes is now reduced to 3-5 seconds. Most importantly, setting more codes will not increase the time too much. This is a temporary solution, but it is better than no solution. We can use the
InstalledCode
in the future when we fix the thread problem.