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

deps: limit dependency ranges #54

Merged
merged 9 commits into from
Jun 20, 2024
Merged

deps: limit dependency ranges #54

merged 9 commits into from
Jun 20, 2024

Conversation

anhuong
Copy link
Collaborator

@anhuong anhuong commented Feb 23, 2024

Limit dependency ranges to automatic minor upgrades, must manually check for major upgrades to see what breaking changes are included. Matches current versions that are being used

- automatic minor upgrades, must check for major

Signed-off-by: Anh-Uong <anh.uong@ibm.com>
- splitup flash-attn and core requirements

Signed-off-by: Anh-Uong <anh.uong@ibm.com>
requirements.txt Outdated Show resolved Hide resolved
@anhuong
Copy link
Collaborator Author

anhuong commented Mar 6, 2024

relates/depends on #68

requirements.txt Outdated
sentencepiece>=0.1.99,<2.0
tokenizers>=0.13.3,<1.0
tqdm>=4.66.2,<5.0
trl>=0.7.10,<1.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

for libraries that are not yet 1.0, I think it is not guaranteed that there won't be any API breaking change between subversions, so it is probably better to be more conservative about their versions in range.

Copy link
Collaborator

Choose a reason for hiding this comment

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

applies less for accelerate though, since that library just never went 1.0 but is very popular, so unlikely to make such changes now without big splash.

@gkumbhat gkumbhat mentioned this pull request Mar 12, 2024
2 tasks
Signed-off-by: Anh-Uong <anh.uong@ibm.com>
Signed-off-by: Anh-Uong <anh.uong@ibm.com>
Signed-off-by: Anh-Uong <anh.uong@ibm.com>
Signed-off-by: Anh-Uong <anh.uong@ibm.com>
pyproject.toml Show resolved Hide resolved
@anhuong
Copy link
Collaborator Author

anhuong commented May 28, 2024

@gkumbhat do you mind giving this another review? I agree with your point that

for libraries that are not yet 1.0, I think it is not guaranteed that there won't be any API breaking change between subversions, so it is probably better to be more conservative about their versions in range.

but I'm not sure what more conservative version to set as the upper limit, any suggestions?

@anhuong
Copy link
Collaborator Author

anhuong commented May 28, 2024

@tedhtchang if you can also give this another review pass, many thanks!

Copy link
Collaborator

@tedhtchang tedhtchang left a comment

Choose a reason for hiding this comment

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

@anhuong Could you rebase.
For the important packages, I think we could set more conservative limit for those lower than 1.0.
In new PR we can figure out how we freeze packages versions in case we need to create older commit.

pyproject.toml Outdated
"fire",
"simpleeval",
"numpy>=1.26.4,<2.0",
"accelerate>=0.20.3,<1.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps set more conservative 0.4 (current 0.31) since it's lower than 1.0

pyproject.toml Outdated
"accelerate>=0.20.3,<1.0",
"transformers>=4.34.1,<5.0,!=4.38.2",
"torch>=2.2.0,<3.0",
"sentencepiece>=0.1.99,<1.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps set more conservative limit say 0.3 (current 0.2). This package updates very slow.

pyproject.toml Outdated
"sentencepiece>=0.1.99,<1.0",
"tokenizers>=0.13.3,<1.0",
"tqdm>=4.66.2,<5.0",
"trl>=0.7.10,<1.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps 0.10.0 current is 0.9.4

pyproject.toml Outdated
"tokenizers>=0.13.3,<1.0",
"tqdm>=4.66.2,<5.0",
"trl>=0.7.10,<1.0",
"peft>=0.8.0,<1.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps set limit to 0.13 current 0.11.1

Signed-off-by: Anh-Uong <anh.uong@ibm.com>
pyproject.toml Show resolved Hide resolved
Signed-off-by: Anh-Uong <anh.uong@ibm.com>
tedhtchang
tedhtchang previously approved these changes Jun 20, 2024
Copy link
Collaborator

@tedhtchang tedhtchang left a comment

Choose a reason for hiding this comment

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

/LGTM

Signed-off-by: Anh-Uong <anh.uong@ibm.com>
Copy link
Collaborator

@tedhtchang tedhtchang left a comment

Choose a reason for hiding this comment

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

/LGTM

@tedhtchang tedhtchang merged commit 07caff5 into main Jun 20, 2024
9 checks passed
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