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

requirements.txt: Remove torch upper bound #423

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

prarit
Copy link

@prarit prarit commented Dec 1, 2024

instructlab has removed the upper bound for torch as it was causing problems with testing of new ROCm and Cuda releases.

For example, the two most current AMD ROCm releases require a version of torch greater than or equal to 2.5.1.

Remove the torch upper bound.

Signed-off-by: Prarit Bhargava [email protected]

instructlab has removed the upper bound for torch as it was causing
problems with testing of new ROCm and Cuda releases.

For example, the two most current AMD ROCm releases require a version of
torch greater than or equal to 2.5.1.

Remove the torch upper bound.

Signed-off-by: Prarit Bhargava <[email protected]>
@mergify mergify bot added ci-failure dependencies Pull requests that update a dependency file labels Dec 1, 2024
@bbrowning
Copy link
Contributor

You'll see in instructlab/instructlab#2528 that the main instructlab repo updated its torch version range to torch>=2.3.0,<2.5.0. I don't think we want an entirely unbounded upper range, as we've had multiple cases already where unbounded upper dependency ranges cause newer things to get pulled in later on that didn't exist when we cut a release and it breaks people installing a specific version of InstructLab in the future. We could raise the limit to something we've tested, as long as we have confidence that the minor torch releases in the range work. So, if you for example need it to be >=2.3.0, < 2.6.0 that could work as long as Torch 2.3, 2.4, 2.5 work with InstructLab.

When the e2e tests ran on this PR, they still used Torch 2.4.x so something else in the dependency chain must be keeping us on Torch 2.4.x.

@mergify mergify bot removed the ci-failure label Dec 2, 2024
@bbrowning
Copy link
Contributor

Should we close this PR now? We loosened the upper bound and cut an SDG 0.6.3 release with the loosened upper bound that allows PyTorch 2.5 to be used. For now I'd prefer to continue to raise the upper bound as we test compatible PyTorch versions versus just removing it entirely. However, if there's a good reason to remove the upper bound entirely I'm open to being convinced of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants