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

conda env instructions #335

Merged
merged 2 commits into from
Apr 20, 2021
Merged

conda env instructions #335

merged 2 commits into from
Apr 20, 2021

Conversation

anuragprat1k
Copy link
Contributor

Description

image

Fixes # (issue)

Type of change

Please check the options that are relevant.

  • Bug fix (non-breaking change that fixes an issue)
  • Proposes a change (non-breaking change that isn't necessarily a bug)
  • Refactor
  • New feature (non-breaking change that adds a new functionality)
  • Breaking change (fix or feature that would break some existing functionality downstream)
  • This is a unit test
  • Documentation only change

Type of requested review

  • I want a thorough review of the implementation.
  • I want a high level review.
  • I want a deep design review.

Checklist:

  • I have performed manual end-to-end testing of the feature in my environment.
  • I have added Docstrings and comments to the code.
  • I have made changes to existing documentation where needed.
  • I have added tests that show that the PR is functional.
  • New and existing unit tests pass locally with my changes.
  • I have added relevant collaborators to review the PR before merge.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 19, 2021
@anuragprat1k anuragprat1k requested a review from soumith April 19, 2021 20:10
Copy link
Member

@soumith soumith left a comment

Choose a reason for hiding this comment

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

if we are going the route of mandating conda, which I really dont mind, then we should also move the installation of PyTorch into the conda install so that it isn't brittle around the version of CUDA installed.

Approving this for now, will move some of the requirements.txt stuff into conda commands myself.

In the "Dependencies", now you have to specify Anaconda / Miniconda

@anuragprat1k
Copy link
Contributor Author

if we are going the route of mandating conda, which I really dont mind, then we should also move the installation of PyTorch into the conda install so that it isn't brittle around the version of CUDA installed.

Approving this for now, will move some of the requirements.txt stuff into conda commands myself.

In the "Dependencies", now you have to specify Anaconda / Miniconda

Currently, the dependencies say "(Anaconda recommended)". Going to leave it at that. Agree with your idea of moving stuff from requirements.txt for stronger consistency.

@anuragprat1k anuragprat1k merged commit ea07b81 into main Apr 20, 2021
@anuragprat1k anuragprat1k deleted the anuragprat1k-patch-4 branch April 20, 2021 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants