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

Improved installation instructions #69

Merged
merged 5 commits into from
Dec 13, 2023

Conversation

thorstenwagner
Copy link
Contributor

@thorstenwagner thorstenwagner commented Dec 12, 2023

Thanks for this package :-)

I would like to suggest some changes:

  1. Install cupy via conda-forge, otherwise it will try to install it from source, which often fails.
  2. When creating the cuda env, base it on cuda 11.8. This way your users are not forced to have the latest cuda driver installed.
  3. Improved compatibility with zsh: pip install '.[plotting]'. Without the quotes it will crash with zsh.

Best,
Thorsten

Copy link
Collaborator

@McHaillet McHaillet left a comment

Choose a reason for hiding this comment

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

Hi Thorsten,

Thanks a lot for the PR for improved install instructions!

A few suggested changes:

  • I think its a good idea to change the default to the prebuild cupy with cuda-toolkit from conda-forge, but also wanted to offer the cupy build if people are comfortable with that.
  • A few things I now realised that might be good to add in ;)

Best!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
thorstenwagner and others added 3 commits December 13, 2023 12:27
Co-authored-by: Marten <58044494+McHaillet@users.noreply.github.com>
Co-authored-by: Marten <58044494+McHaillet@users.noreply.github.com>
Co-authored-by: Marten <58044494+McHaillet@users.noreply.github.com>
Copy link
Collaborator

@McHaillet McHaillet left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM 👍

@McHaillet McHaillet merged commit d7ef10b into SBC-Utrecht:main Dec 13, 2023
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.

None yet

2 participants