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 and fix conda environment construction script #262

Merged
merged 4 commits into from
Mar 24, 2022
Merged

Conversation

bwohlberg
Copy link
Collaborator

  • Remove options for installation with GPU support
  • Remove requirement that script be run from repo root directory
  • Updates to restore functionality broken by recent changes in the repo

@bwohlberg bwohlberg added the developer Developer environment: issues related to CI, git, etc. label Mar 23, 2022
@codecov
Copy link

codecov bot commented Mar 24, 2022

Codecov Report

Merging #262 (96b2043) into main (1c71bca) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #262   +/-   ##
=======================================
  Coverage   93.61%   93.61%           
=======================================
  Files          49       49           
  Lines        3541     3541           
=======================================
  Hits         3315     3315           
  Misses        226      226           
Flag Coverage Δ
unittests 93.61% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c71bca...96b2043. Read the comment docs.

echo "Activate the conda environment with the command"
echo " conda activate $ENVNM"
echo "The environment can be deactivated with the command"
echo " conda deactivate"
echo
echo "Jax installed without GPU support. To avoid warning messages,"
Copy link
Contributor

Choose a reason for hiding this comment

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

This means if GPU support is required then you should not use this script?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, it means that manual action is required after use of the script. I had assumed that this was implied by the following sentence "To include GPU support ...", but perhaps it's not as clear as I had thought?

Copy link
Contributor

Choose a reason for hiding this comment

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

It was not clear to me. Let's make it more clear.

Also in the .rst file we could make it something like To include GPU support, follow the jax installation instructions after running the installation script.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@bwohlberg bwohlberg merged commit 63ea350 into main Mar 24, 2022
@bwohlberg bwohlberg deleted the brendt/conda branch March 24, 2022 21:39
Copy link
Contributor

@Michael-T-McCann Michael-T-McCann left a comment

Choose a reason for hiding this comment

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

Always like to see the negative lines of code added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer Developer environment: issues related to CI, git, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants