-
Notifications
You must be signed in to change notification settings - Fork 41
pytorch uenv v2.8.0 #296
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
base: main
Are you sure you want to change the base?
pytorch uenv v2.8.0 #296
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
1 similar comment
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
docs/guides/storage.md
Outdated
| # in this case we explicitly select python 3.12 | ||
| uv venv -p 3.12 --relocatable --link-mode=copy /dev/shm/sqfs-demo/.venv | ||
| # in this case we explicitly select the python interpreter from the uenv view | ||
| uv venv -p /user-environment/env/default/bin/python --system-site-packages --relocatable --link-mode=copy /dev/shm/sqfs-demo/.venv |
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.
Could this use -p $(which python) --system-site-packages to be a little bit more portable?
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.
Also, can we quickly explain why --system-site-packages is used?
lukasgd
left a comment
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.
Thanks for the PR! When reviewing this, I felt that we have general instructions for building virtual environments on top of base environments (be it uenv or containers) in multiple locations (Connecting to Alps > Jupyter, Applications and Frameworks > ML > Pytorch, Guides > Storage > Many small files vs. HPC File Systems) that are not necessarily that easy to find for a user. Would it make sense to work on merging these instructions to a single section under e.g. Environments?
docs/guides/storage.md
Outdated
| unset PYTHONPATH | ||
| export PYTHONUSERBASE=/user-environment/env/default |
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.
This will probably break the Jupyterlab setup
cscs-docs/docs/access/jupyterlab.md
Line 141 in b8f83cc
| --user --name="<kernel-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.
I think this should work in all cases - can you confirm @lukasgd ?
python -m ipykernel install --sys-prefix --name mykernel ${VIRTUAL_ENV:+--env PATH $PATH --env VIRTUAL_ENV $VIRTUAL_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.
Did you test this? I'm not sure if this works with just a uenv when not using a virtual env on top. Also, would kernels still be conveniently listed for selection when opening Jupyterlab (as they are with --user)? Adding cc @rsarm @twrobinson if you'd like to comment.
|
|
||
| # unset PYTHONPATH and set PYTHONUSERBASE to avoid conflicts | ||
| unset PYTHONPATH | ||
| export PYTHONUSERBASE=/user-environment/env/default |
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.
Same as above regarding Jupyterlab
docs/software/ml/pytorch.md
Outdated
| ??? bug "Python packages from uenv view shadowing those in a virtual environment" | ||
| Some uenv views set the `PYTHONPATH` environment variable and/or do not set the `PYTHONUSERBASE` environment variable. | ||
| This can lead to unexpected behavior when using Python virtual environments on top of the uenv, as the packages installed in the uenv view may take precedence over those in the virtual environment. | ||
| A possible workaround is to unset the `PYTHONPATH` and set the `PYTHONUSERBASE` environment variables, as described in the [Python virtual environments with uenv guide][ref-guides-storage-venv]: | ||
| ```bash | ||
| export PYTHONPATH="$(python -c 'import site; print(site.getsitepackages()[0])'):$PYTHONPATH" | ||
| unset PYTHONPATH | ||
| export PYTHONUSERBASE=/user-environment/env/default | ||
| ``` | ||
| It is recommended to apply this workaround if you are constrained by a Python package version installed in the uenv that you need to change for your application. | ||
| It is recommended to apply this workaround if you are constrained by a Python package version installed in the uenv view that you need to change for your application. |
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.
See above for conflict with Jupyterlab docs. We've covered this issue in multiple places in the docs, including
cscs-docs/docs/software/ml/pytorch.md
Line 555 in b8f83cc
| ??? bug "Python packages from uenv shadowing those in a virtual environment" |
cscs-docs/docs/access/jupyterlab.md
Line 132 in b8f83cc
| ??? bug "Python packages from uenv shadowing those in a virtual environment" |
Overall, I think it would be good to have a consistent recommendation.
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.
Is it time to create a central "using Python with uenv" documentation that these pages can refer to?
That might be beyond the scope of this update - but we can do it later this quarter.
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, or overall central "How to extend Python base environments with venvs" also including the CE, cf. #296 (review)
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
preview available: https://docs.tds.cscs.ch/296 |
|
uenv PR: eth-cscs/alps-uenv#258