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.

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

Update Euler Guide #986

Merged
merged 12 commits into from
Feb 11, 2025
Merged

Update Euler Guide #986

merged 12 commits into from
Feb 11, 2025

Conversation

emanuel-schmid
Copy link
Collaborator

@emanuel-schmid emanuel-schmid commented Dec 12, 2024

Changes proposed in this PR:

  • Update the Euler Guide to the new OS

PR Author Checklist

PR Reviewer Checklist

Copy link
Collaborator

@sarah-hlsn sarah-hlsn left a comment

Choose a reason for hiding this comment

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

Thanks Emanuel, these instructions are super useful!

I have used the "Climada installation in a virtual environment" and "Run a Jupyter Notebook on Euler" instructions myself, and they work well for me. I have added a few comments which could increase the clarity of the instructions for some users.

I have not implemented the other sections of this tutorial myself, but the instructions seem sufficiently clear for me.

"envname=climada_env\n",
"\n",
"# create environment\n",
"python -m venv --system-site-packages ~/venv/$envname\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"python -m venv --system-site-packages ~/venv/$envname\n",
"python -m venv --system-site-packages ./venv/$envname\n",

Copy link
Collaborator

Choose a reason for hiding this comment

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

~ would direct back to the home directory, which we want to avoid here, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm. Valid question. Not necessarily. the environemnt will contain about 80000 files and occupy about 600M. That is quite suitable for the home directory. For other parts of the file system that's less convenient. The project folder would be an option, but then we have to provide guidance on how to organize virtual environments there.
On the other hand, . is a dangerous folder. Once you logged out you may never find it again (;

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not against installing in the home folder if that works, just above it says "As a general rule: use /cluster/project for installation" so I think the intructions should be consistent between the text and the code

"\n",
"should output something like this:\n",
"# acitvate it\n",
". ~/venv/$envname/bin/activate\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
". ~/venv/$envname/bin/activate\n",
". ./venv/$envname/bin/activate\n",

"\n",
"There are two options. Either install from the downloaded repository (option A), or use a particular released version (option B).\n",
"\n",
"#### option A\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here I would explicitly add the lines to clone the git repository, otherwise it is not clear that the user still has to do that:

git clone https://github.com/CLIMADA-project/climada_python.git
cd /cluster/project/climate/$USER/climada_python
git checkout develop

"python -c 'import climada; print(climada.__file__)'\n",
"module unload proj\n",
"\n",
"EOF\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Potentially add something to the effect of "If you encounter a No such directory error when running the lines above, run mkdir -p /cluster/home/shuelsen/.config/euler/jupyterhub/ and try rerunning the lines above"

doc/guide/Guide_Euler.ipynb Outdated Show resolved Hide resolved
doc/guide/Guide_Euler.ipynb Outdated Show resolved Hide resolved
"```\n",
"envname=climada_env\n",
"\n",
"# create environment\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"# create environment\n",
"# navigate to your project directory\n",
"cd /cluster/project/climate/$USER\n",
"# create environment\n",

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

actually one doesn't need to be there 🤷

doc/guide/Guide_Euler.ipynb Outdated Show resolved Hide resolved
doc/guide/Guide_Euler.ipynb Outdated Show resolved Hide resolved
@@ -45,7 +45,7 @@
"metadata": {},
"source": [
"\n",
"## Pre-installed version of Climada\n",
"## Climada installation in a virtual environment\n",
"\n",
"Climada is pre-installed and available in the default pip environment of Euler."
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought this is no longer the case with the new Euler set up? If not, best remove. If it is, best add a clarification under which circumstances users should use the default installation and when to follow the instructions below.

@emanuel-schmid
Copy link
Collaborator Author

🙌 many thanks for the review @sarah-hlsn

@emanuel-schmid emanuel-schmid merged commit 65cd920 into develop Feb 11, 2025
3 of 12 checks passed
@emanuel-schmid emanuel-schmid deleted the feature/euler_guide_update branch February 11, 2025 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants