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

Make pip installable #24

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

MilesCranmer
Copy link

This enables you to install it with:

pip install git+https://github.com/ldeecke/gmm-torch

rather than cloning and copying the code into a codebase

@ldeecke
Copy link
Owner

ldeecke commented May 8, 2022

Makes sense and looks good in principle — thanks for the PR! 👍

Some suggestions:

  • could rename from gmm_torch/gmm.py to gmm/model.py
  • {example,test.py} seem to loose their functionality
  • what is the rationale for dropping torch from requirements?

@MilesCranmer
Copy link
Author

Regarding dropping torch, I’ve noticed it will often mess up installations of torch when you put it in setup. For example, if you’ve installed the GPU version, the setup.py might make pip switch it to the CPU version. So it’s better to assume someone will install pytorch manually using the steps on https://pytorch.org/get-started/locally/ which vary by platform and accelerator.

@Linux-cpp-lisp
Copy link

It would be great to have this available in PyPI; it would make it much easier to use and ensure that downstream packages are getting any updates or bugfixes. It's possible (and really quite easy) to set up GitHub actions to upload to PyPI every time you make a release in GitHub, see for example in one of my packages here: https://github.com/Linux-cpp-lisp/opt_einsum_fx/blob/main/.github/workflows/release.yml

Also just seconding @MilesCranmer 's point about torch as a dependency... I've never seen pip do anything sane or helpful with it, but it's definitely ruined a lot of environments and wasted a lot of bandwidth.

@AndreasGerken
Copy link

I would also love to have this as pip installable. It's very easy and saves each users setup time. If it would be in PyPi it would be even more amazing and it would probably find even more users.

By now there is no specific torch-gpu version anymore (was there one once?) so there should not be any fear of messing up environments.

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.

4 participants