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.

Sign up for GitHub

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

Set LD_LIBRARY_PATH on mamba activate #1496

Merged
merged 4 commits into from
Sep 12, 2023
Merged

Set LD_LIBRARY_PATH on mamba activate #1496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c24b9cb
Set LD_LIBRARY_PATH on mamba activate
Sep 12, 2023
21339e9
Rename libcudart_activate.sh to sleap_activate.sh in docs
Sep 12, 2023
94efb5f
Remove comments from environment.yml
Sep 12, 2023
c73d465
Merge branch 'develop' into liezl/set-LD_LIBRARY_PATH-on-activate
roomrys Sep 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .conda/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ This folder defines the conda package build for Linux and Windows. There are run
To build, first go to the base repo directory and install the build environment:

```
conda env create -f environment_build.yml -n sleap_build && conda activate sleap_build
mamba env create -f environment_build.yml -n sleap_build && conda activate sleap_build
```

And finally, run the build command pointing to this directory:
Expand All @@ -15,7 +15,7 @@ conda build .conda --output-folder build -c conda-forge -c nvidia -c https://con
To install the local package:

```
conda create -n sleap_0 -c conda-forge -c nvidia -c ./build -c https://conda.anaconda.org/sleap/ -c anaconda sleap=x.x.x
mamba create -n sleap_0 -c conda-forge -c nvidia -c ./build -c https://conda.anaconda.org/sleap/ -c anaconda sleap=x.x.x
```

replacing x.x.x with the version of SLEAP that you just built.
10 changes: 9 additions & 1 deletion .conda/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,12 @@ pip install --no-cache-dir -r ./requirements.txt

# Install sleap itself. This does not install the requirements, but will list which
# requirements are missing (see "install_requires") when user attempts to install.
python setup.py install --single-version-externally-managed --record=record.txt
python setup.py install --single-version-externally-managed --record=record.txt

# Copy the activate scripts to $PREFIX/etc/conda/activate.d.
# This will allow them to be run on environment activation.
export CHANGE=activate

mkdir -p "${PREFIX}/etc/conda/${CHANGE}.d"
ls "${RECIPE_DIR}"
cp "${RECIPE_DIR}/${PKG_NAME}_${CHANGE}.sh" "${PREFIX}/etc/conda/${CHANGE}.d/${PKG_NAME}_${CHANGE}.sh"
Comment on lines +17 to +23
Copy link

@coderabbitai coderabbitai bot Sep 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This new block of code is responsible for copying the activation scripts to a specific directory. This will allow them to be run when the environment is activated. However, there's no error handling in case the copy operation fails. It would be beneficial to add some error checking here to ensure that the script has been copied successfully.

- cp "${RECIPE_DIR}/${PKG_NAME}_${CHANGE}.sh" "${PREFIX}/etc/conda/${CHANGE}.d/${PKG_NAME}_${CHANGE}.sh"
+ cp "${RECIPE_DIR}/${PKG_NAME}_${CHANGE}.sh" "${PREFIX}/etc/conda/${CHANGE}.d/${PKG_NAME}_${CHANGE}.sh" || {
+   echo "Failed to copy ${PKG_NAME}_${CHANGE}.sh";
+   exit 1;
+ }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The build just fails and gives us an error message if these lines end in failure.

Copy link

Choose a reason for hiding this comment

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

@roomrys While it's true that the build will fail and provide an error message if these lines end in failure, adding explicit error handling can still be beneficial. It allows you to control the error message that is displayed, making it more informative and specific to the operation that failed. This can help with troubleshooting and understanding exactly what went wrong.

For example, without explicit error handling, you might get a generic "No such file or directory" error. With explicit error handling, you can provide a message like "Failed to copy ${PKG_NAME}_${CHANGE}.sh", which immediately tells you that the script failed to copy.

Additionally, explicit error handling allows you to control the exit code of the script. This can be useful if other scripts or systems are relying on specific exit codes for certain types of failures.

Here's how you could modify the code to include explicit error handling:

cp "${RECIPE_DIR}/${PKG_NAME}_${CHANGE}.sh" "${PREFIX}/etc/conda/${CHANGE}.d/${PKG_NAME}_${CHANGE}.sh" || {
  echo "Error: Failed to copy ${PKG_NAME}_${CHANGE}.sh to ${PREFIX}/etc/conda/${CHANGE}.d/";
  exit 1;
}

This way, if the copy operation fails, the script will print a specific error message and then exit with a status of 1.

4 changes: 4 additions & 0 deletions .conda/sleap_activate.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/bin/sh

# Help CUDA find GPUs!
export LD_LIBRARY_PATH=$CONDA_PREFIX/lib:$LD_LIBRARY_PATH
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you sure this will work? I thought the cuda binaries were stored in the $CONDA_PREFIX/envs/{env_name}/lib or something.

Like, one blocker for upgrading to newer tensorflow was that we couldn't dynamically set this since users would have different env names.

If it's really in $CONDA_PREFIX/lib and it's not just hardcoding the path from the computer you built in (would Jinja do this?), then that's awesome and would allow us to move up to tf 2.9 I think!

If it doesn't work, then it might be ok to just include the extra line that people have to run in the install docs?

Copy link
Collaborator Author

@roomrys roomrys Sep 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The $CONDA_PREFIX includes the environment name:

(nb4) talmolab@talmolab-01-ubuntu:~/sleap-estimates-animal-poses/pull-requests/sleap$ echo $CONDA_PREFIX
/home/talmolab/micromamba/envs/nb4

!!!

Copy link
Collaborator Author

@roomrys roomrys Sep 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

If it's really in $CONDA_PREFIX/lib and it's not just hardcoding the path from the computer you built in (would Jinja do this?), ...

I think that might be the point of using cp now that you mention it (instead of echo ... >> ...).

sleap/.conda/build.sh

Line 23 in 21339e9

cp "${RECIPE_DIR}/${PKG_NAME}_${CHANGE}.sh" "${PREFIX}/etc/conda/${CHANGE}.d/${PKG_NAME}_${CHANGE}.sh"

...then that's awesome and would allow us to move up to tf 2.9 I think!

😨

25 changes: 25 additions & 0 deletions docs/installation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,31 @@ pip install tensorflow==2.6.3
```
````

````{note}
If you are on Linux, have a NVIDIA GPU, and are having trouble utilizing your GPU:

```bash
W tensorflow/stream_executor/platform/default/dso_loader.cc:64 Could not load dynamic
library 'libcudart.so.11.0'; dlerror: libcudart.so.11.0: cannot open shared object
file: No such file or directory
```

then activate the environment:

```bash
mamba activate sleap
```

and run the commands:
```bash
mkdir -p $CONDA_PREFIX/etc/conda/activate.d
echo '#!/bin/sh' >> $CONDA_PREFIX/etc/conda/activate.d/sleap_activate.sh
echo 'export LD_LIBRARY_PATH=$CONDA_PREFIX/lib:$LD_LIBRARY_PATH' >> $CONDA_PREFIX/etc/conda/activate.d/sleap_activate.sh
source $CONDA_PREFIX/etc/conda/activate.d/sleap_activate.sh
```
These commands only need to be run once and will subsequently run automatically upon activating your `sleap` environment.
````

## Upgrading and uninstalling

We **strongly recommend** installing SLEAP in a fresh environment when updating. This is because dependency versions might change, and depending on the state of your previous environment, directly updating might break compatibility with some of them.
Expand Down
1 change: 1 addition & 0 deletions environment.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,4 @@ dependencies:

- pip:
- "--editable=.[conda_dev]"