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

add support for newer pip and CUDA computation versions #151

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

jamesobutler
Copy link
Contributor

@jamesobutler jamesobutler commented Dec 7, 2024

Pip 23.3.x support
Closes #146
Closes #147
Closes #148

Pip 24.0.x support
Closes #149

This PR includes some general maintenance updates for the github actions workflow to ultimately test the newer pip version. A dependabot workflow has been added so that github actions can be automatically updated and PRs issued. Turn on dependabot alerts at https://github.com/pmeier/light-the-torch/settings/security_analysis.

Python versions have been updated to drop Python 3.7 and 3.8, but to specifically support Python 3.12. This goes along with supporting newer CUDA 12.x computational platforms.

@jamesobutler jamesobutler force-pushed the support-newer-versions branch 3 times, most recently from 85a428a to e9d2802 Compare December 7, 2024 15:39
Copy link

codecov bot commented Dec 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.5%. Comparing base (19e77b7) to head (9b38b69).

Additional details and impacted files
@@           Coverage Diff           @@
##            main    #151     +/-   ##
=======================================
- Coverage   78.9%   78.5%   -0.4%     
=======================================
  Files          6       5      -1     
  Lines        384     378      -6     
=======================================
- Hits         303     297      -6     
  Misses        81      81             
Flag Coverage Δ
unit 78.5% <100.0%> (-0.4%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Latest torch nightly as of creating this commit is https://github.com/pytorch/pytorch/blob/f84e533a2cb89a42c021dce7d22af7d5bd5f5ac1/setup.py#L235 which requires Python 3.9+.

Current latest stable version of torch (2.5.1) still supports Python 3.8. See https://github.com/pytorch/pytorch/blob/v2.5.1/setup.py#L217.
@jamesobutler jamesobutler force-pushed the support-newer-versions branch 2 times, most recently from 8e19baa to 228ae8d Compare December 7, 2024 18:01
@jamesobutler jamesobutler changed the title add support for pip==23.3 add support for newer pip versions Dec 7, 2024
jamesobutler and others added 16 commits December 8, 2024 10:47
Python 3.7 went end-of-life on June 27th 2023. See https://endoflife.date/python.

There is not a Python 3.7 for arm64 using the macos-latest runner for light-the-torch tests.yml workflow.
Bumps [actions/checkout](https://github.com/actions/checkout) from 2 to 4.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v2...v4)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [actions/cache](https://github.com/actions/cache) from 2 to 4.
- [Release notes](https://github.com/actions/cache/releases)
- [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md)
- [Commits](actions/cache@v2...v4)

---
updated-dependencies:
- dependency-name: actions/cache
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 3 to 5.
- [Release notes](https://github.com/codecov/codecov-action/releases)
- [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md)
- [Commits](codecov/codecov-action@v3...v5)

---
updated-dependencies:
- dependency-name: codecov/codecov-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
light-the-torch specifies support for python 3.11 in pyproject.toml so lets run unit tests on it as well.
This change should have been included when importlib.metadata import was changed during recent commit that dropped Python 3.7 support.

This change was automatically applied by running:

python -m pre_commit run --all-files
This should have been completed in the recent commit that dropped Python 3.7 support.
This was automatically applied by running:

python -m pre_commit autoupdate

Additional dependencies such as black and usort were updated manually.
This change was automatically applied by running:

python -m pre_commit run --all-files
This supports triggering a workflow based on any individual commit or branch. This provides flexibility when testing a wide array of commits
@jamesobutler jamesobutler force-pushed the support-newer-versions branch from 4132597 to 642a793 Compare December 8, 2024 15:50
@jamesobutler jamesobutler changed the title add support for newer pip versions add support for newer pip and CUDA computation versions Dec 8, 2024
Python 3.8 went end-of-life on October 7th 2024. See https://endoflife.date/python.

Latest Pytorch 2.5 specified support for Python 3.8 in its setup.py, but no Python 3.8 whl files were created. Torch developers resolved their mistake clarifying Python 3.8 support had indeed been dropped. See pytorch/pytorch@5c49db9.
@jamesobutler jamesobutler force-pushed the support-newer-versions branch from 772a824 to 9b38b69 Compare December 8, 2024 16:32
@jamesobutler
Copy link
Contributor Author

@pmeier Thanks for your original work in creating light-the-torch. The 3D Slicer community has been using it for several years to help users install the appropriate CUDA whl through the use of the SlicerPyTorch extension. The 3D Slicer project will soon be updating the embedded python version to something newer such as Python 3.12 and so these updates in this PR will help provide official support and testing for light-the-torch for newer python and newer CUDA computation platforms for latest PyTorch such as CUDA 12.4. It would be great if you can take a look at this PR and ultimately "Rebase and Merge" to integrate the collection of individual commits in this PR into the main branch and publish a new version (0.8.0) to PyPI. Thanks!

@pmeier
Copy link
Owner

pmeier commented Dec 9, 2024

@jamesobutler Thanks a ton for the PR! I'll find some time this week (I promise!) to merge this and push a release.

To give some context: I'm no longer a PyTorch dev / power user and thus have very little use for light-the-torch in my day-to-day. And with that maintaining this project became more of a burden to me. However, the fact that it still works and has roughly 1k downloads per week after being unmaintained for a good year, is a testament that this project shouldn't be archived.

Since you basically did all the maintainer work in this PR, would you like to have maintainer rights on this repository so that you can push fixes yourself in the future?

@jamesobutler
Copy link
Contributor Author

jamesobutler commented Dec 9, 2024

I appreciate your help on this!

Since you basically did all the maintainer work in this PR, would you like to have maintainer rights on this repository so that you can push fixes yourself in the future?

Yes, that would be great!

To give some context: I'm no longer a PyTorch dev / power user and thus have very little use for light-the-torch in my day-to-day. And with that maintaining this project became more of a burden to me.

I can discuss with the other Slicer main developers (e.g. @lassoan, also a light-the-torch contributor, see here), but would you potentially consider transferring the repo to the https://github.com/Slicer GitHub organization (or to me to then transfer to the Slicer org)? Any links to https://github.com/pmeier/light-the-torch would automatically redirect to a migrated https://github.com/Slicer/light-the-torch location. Then myself and other Slicer main developers could continue maintenance of the project. There are probably several of us that would like to keep this going for the Slicer community. We would additionally add props to you in the README with a history section indicating the work was primarily under your account and that it was later transferred to the Slicer organization to carry maintenance forward. If you'd like to keep this repo under your GitHub name, I totally understand. Only brought this up if this repo has become a burden to the point you want to pass it to another person/organization. Otherwise we can also work within the bounds of getting write access to this repo and get access for other potential maintainers from the Slicer community.

@pmeier
Copy link
Owner

pmeier commented Dec 14, 2024

but would you potentially consider transferring the repo to the https://github.com/Slicer GitHub organization (or to me to then transfer to the Slicer org)?

I'm not exactly sure why, that hit hard reading it the first time. I didn't like it at all. But the longer I thought about it, the more sense it makes to me. I wanted to have this PR finished by no later than Thursday and here I am sitting on a Saturday night, because I simply didn't find any time earlier. Coupled with the fact that I'm no longer a user of this project, it makes little sense to keep me as maintainer and by proxy to keep this as my repository. So, yes, I'm happy to transfer it anywhere of your choosing. I'd prefer under the Slicer umbrella to make this a proper community project, but I'm ok with either.

Could you open an issue to discuss the details rather than doing it on this PR?

Copy link
Owner

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

Just some minor comments. Can merge on short notice after fix. I'll push v0.8.0 afterwards. Thanks a ton James!

Comment on lines +1 to +5
# To get started with Dependabot version updates, you'll need to specify which
# package ecosystems to update and where the package manifests are located.
# Please see the documentation for all configuration options:
# https://docs.github.com/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file

Copy link
Owner

Choose a reason for hiding this comment

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

Let's not keep boilerplate comments

Suggested change
# To get started with Dependabot version updates, you'll need to specify which
# package ecosystems to update and where the package manifests are located.
# Please see the documentation for all configuration options:
# https://docs.github.com/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file

Comment on lines +10 to +11
# Allows running this workflow manually from the Actions tab
workflow_dispatch:
Copy link
Owner

Choose a reason for hiding this comment

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

Just my suggestion given that this will soon no longer my responsibility:

This comment is kinda unnecessary. It is correct, but having this comment there begs the question what is special about this one.

Suggested change
# Allows running this workflow manually from the Actions tab
workflow_dispatch:
workflow_dispatch:

I'm not going to add this everywhere, but the same comment pops up multiple times.

Comment on lines +78 to +79
- cu121
- cu124
Copy link
Owner

Choose a reason for hiding this comment

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

The matrix of CI workflows is already massive. I'd keep it to a single CUDA 11 and 12.

@@ -9,7 +9,7 @@ repos:
- id: end-of-file-fixer

- repo: https://github.com/pre-commit/mirrors-prettier
rev: v2.5.1
rev: v4.0.0-alpha.8
Copy link
Owner

Choose a reason for hiding this comment

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

Upgrading here is fine, but do you really want to depend on an alpha version? Plus, according to the README of the action, this is no longer maintained. Meaning, it will never reach a stable v4. I'd prefer to keep a stable v3 here.

@@ -18,9 +18,9 @@ repos:
- yaml

- repo: https://github.com/omnilib/ufmt
rev: v1.3.2
rev: v2.8.0
Copy link
Owner

Choose a reason for hiding this comment

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

Probably want to switch to ruff in the near future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants