Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Migrate build data to pyproject.toml #1078
Migrate build data to pyproject.toml #1078
Changes from 3 commits
4454fc0
7bdc6e5
11142a3
636c62d
f9c2cc5
699f505
00497ab
d23be94
48ede4a
9fae8ee
9a6794b
399a494
0101f2f
72342b5
11165f8
646a86a
5d6ce68
c5fcfb8
19dcbd7
fd725cb
98f73d5
a04cd4d
da2dc69
a0ae460
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need any version constraint at all? Maybe at least a minimum version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do think we should have at least minimums, but I'm not sure what they should be. I took these runtime dependencies out of
setup.py
just as they were.If it were up to me, I would pin
torch>=1.13.0
on the low end for all platforms/backends assuming everything works. Why?torch==1.13.1+cu117
on py3.9** In general, all of the Python 3.9+ containers with PyTorch appear to be torch>=1.13.0
But it could also be a little more permissive for some environments; e.g. transformers recently adopted a constraint of
torch>=1.11,!=1.12.0
in huggingface/transformers#28207, which was also adopted in UKPLab/sentence-transformers#2432.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO (and I recognize this might ba a bit controversial) we should set minimum to what we test on, then pin to these when we test. I don't think we should overstretch to keep supporting old versions of dependencies at the expense of stability or complexity. So I would say ideally we set minimum to what we currently build on and either no upper bound or < next major.
As a user of any package I prefer to know what works or not and not having to be the guinea pig.
But this is my personal opinion, not sure what the rest of you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we would have a CI setup that could test a wider matrix so we can be confident about how we pin these, for sure.
For me part of the point of this
bitsandbytes
project is to try and make things accessible for a wide range of users - so that's across hardware, software, and user's expertise. Since many people, particularly in academia, don't have much control over things like OS, python version, or even torch version, we've got to support a large range in order to make things easy for users. That does come at the cost of maintenance, obviously. Need to find a good balance here.So at least with this change it's being restricted to
torch>=1.11,!=1.12.0
, which is in line withtransformers
. I don't know that all of those versions of torch will work, but it's an incremental step forward compared toinstall_requires=["torch", "numpy"]
, and given the lack of issues around torch versions it does seem likely that it would work.This file was deleted.
This file was deleted.