-
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
minimal changes to allow changing database path #19
Conversation
@dskarls these minimal changes allow for everything the KDP does (at least in the process of pipeline-run-pair, including local dependencies) to be contained within a custom specified directory, which allows it to be run using singularity on a cluster. Also, it seems that the KIM_API_USER_COLLECTION variable wasn't doing anything, I grepped for it and it wasn't referenced anywhere. I believe this doesn't break anything in the main branch, do you see this causing any problems? |
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.
I see no problems whatsoever with the db path customization. As long as the KIM API is handling models properly, you can merge this.
USE_FULL_ITEM_NAMES_IN_REPO=True | ||
|
||
# parent directory where things are run by default | ||
WORKER_RUNNING_PATH=/tmp | ||
|
||
# We install MD/MO/SM to the KIM API user collection | ||
KIM_API_USER_COLLECTION=/home/openkim/.kim-api/ |
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.
KIM_API_USER_COLLECTION
was a special envvar that the KIM API used to inspect when figuring out where to install or load models but it may no longer use it (and it was also only one way to specify that information). As long as you've confirmed that models are installed where you need them to be and are accessible when doing things like pipeline-run-pair
, this should be fine.
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.
Confirmed that the current version of the API doesn't know anything about this variable (both commands return nothing)
ilia@ilia-xps:~/kim-api-2.3.0$ grep KIM_API_USER_COLLECTION README.md
ilia@ilia-xps:~/kim-api-2.3.0$ find -type f | xargs grep KIM_API_USER_COLLECTION
This minimal change allows changing the path of the local database. This is necessary to allow the Docker image to be exported to an operational Singularity container that is able to use the local database (since singularity uses the host filesystem and therefore cannot write to /pipeline/db/). So far,
kimitems install
andpipeline-run-pair
is confirmed working using Singularity. All other necessary paths should be able to be set using a custom file specified with thePIPELINE_ENVIRONMENT_FILE
environment variable passed into the container.