-
Notifications
You must be signed in to change notification settings - Fork 561
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
Installation updates #360
Installation updates #360
Conversation
jnwei
commented
Oct 20, 2023
- Installation scripts now assumes user has their own version of Anaconda, and provides instructions on how to download anaconda separately.
- Upgrades OpenMM package to use OpenmM 7.7.0 and removes patches
- Updates README instructions
…y/openfold into installation-updates
instead of pip - Adds helper line to automatically prepend conda library to $LD_LIBRARY_PATH
…y/openfold into installation-updates Merging changes from different systems
README.md
Outdated
To install: | ||
1. Clone the repository, e.g. `git clone https://github.com/aqlaboratory/openfold.git` | ||
1. From the `openfold` repo: | ||
- Create an [Anaconda/Mamba](https://docs.anaconda.com/free/anaconda/install/index.html) environment, e.g. `conda env create -n openfold_env` |
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.
Would say "conda env create -f environment.yml" here
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.
Done
To activate the environment, run: | ||
|
||
```bash | ||
source scripts/activate_conda_env.sh |
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 guess we shouldn't remove these since people will have the old env, but it's kind of confusing that they refer to the old setup
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.
If we want to keep information/scripts about the old set up, my inclination would be to have a separate section / README for them. These scripts are also more useful if someone is running from a slurm job, so maybe I can move these scripts there and provide some instructions?
|
||
# Grab conda-only packages | ||
export PATH=lib/conda/bin:$PATH | ||
lib/conda/bin/python3 -m pip install nvidia-pyindex |
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.
What was this nvidia pyindex for actually?
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'm not exactly sure. According to https://github.com/nvidia/tensorflow/#install it's supposed to host NVIDIA wheels that are not part of the regular pypi package, but I didn't find anything that requires it? Especially now that we install flash attention using pip rather than building manually from source?
Maybe @gahdritz would know more.
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.
With the current environment.yml, I don't this is used anymore, it seems like it was a specific channel for pulling pip packages.
|
||
python setup.py install | ||
|
||
export LD_LIBRARY_PATH=$CONDA_PREFIX/lib:$LD_LIBRARY_PATH |
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.
Not related to this PR, but could you also add this here to fix that worker issue:
conda env config vars set KMP_AFFINITY=none
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.
noted. Do we know if the KMP_AFFINITY fix was required for all the systems we tested on, or just a few systems?
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.
Multiple systems, three different clusters that I know of.
|
||
# The following will prepend conda library environment to $LD_LIBRARY_PATH | ||
# upon conda library activation | ||
# conda env config vars set LD_LIBRARY_PATH=$CONDA_PREFIX/lib:$LD_LIBRARY_PATH |
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.
Not sure about this, I guess $LD_LIBRARY_PATH could be different on diff nodes, this will override the one set on that node with whatever value it had when you first save the env var.
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.
Yeah, I think you're right, and this is more trouble than it's worth. The first time you deactivate from the environment, it seems to unset the LD_LIBRARY_PATH entirely which can be a bit concerning if you're not expecting that.
…y/openfold into installation-updates