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

Publish Wheels to PyPi #588

Merged
merged 1 commit into from
Dec 14, 2022
Merged

Publish Wheels to PyPi #588

merged 1 commit into from
Dec 14, 2022

Conversation

AbdBarho
Copy link
Contributor

@AbdBarho AbdBarho commented Dec 13, 2022

Related #533 #573 #532 #534 #523 #473 (damn, this is getting out of hand).

A few decisions here:

  • We only upload the wheels built against the latest torch version (1.13) with latest cuda (11.7)
  • the combination 1.13 and 11.6 was removed with the hope that the other version compensates.
  • The 1.12 version is left in for users who still haven't updated yet, but it is not uploaded to pypi.
  • the version string is as discussed in Making built wheels available for install #533 (comment)
    • release candidate with a suffix dev (not rc) to stay consistent with conda
    • we should probably consolidate the version generation to one place

Example run:
https://github.com/AbdBarho/xformers-wheels/actions/runs/3689261975

Published Wheels:
https://test.pypi.org/project/formers/0.0.15.dev376/#files

TODO (for future PRs):

  • Publish on tagged commits
  • Create source distribution

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 13, 2022
Copy link
Contributor

@danthe3rd danthe3rd left a comment

Choose a reason for hiding this comment

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

This looks great! Thanks a lot for putting this together!
Just requested a minor change in the version numbering

Comment on lines 113 to 112
echo "BUILD_VERSION=$version$num_commits" >> ${GITHUB_ENV}
cat ${GITHUB_ENV}
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we should have a system a bit different on pip than conda. We should use pre-releases (eg 0.0.16rc{num_commits}), so that by default, "pip install -U xformers" will install the latest tagged version (0.0.15).
If the user wants a potential unstable pre-release, they should ask for it explicitly ("pip install -U --pre xformers").
What do you think?

It might make things easier to remove the ".dev" suffix from the version.txt file, and instead add it manually where it is needed.

@danthe3rd
Copy link
Contributor

Also we should probably also have a source-distribution, but we can worry about that later

@AbdBarho
Copy link
Contributor Author

@danthe3rd I have added the requested changes.

Example Publish:
https://test.pypi.org/project/formers/0.0.15rc380/

@danthe3rd
Copy link
Contributor

@danthe3rd I have added the requested changes.

Example Publish: https://test.pypi.org/project/formers/0.0.15rc380/

Thanks! Just a last thing - it should be a release candidate for the next version (0.0.16rcXXX here)

@AbdBarho
Copy link
Contributor Author

@danthe3rd so do you mean I should update the version.txt or when computing the version string increment the patch by 1?

@danthe3rd
Copy link
Contributor

Keep version.txt as it is, but when we build for a RC, increment version by 1

@AbdBarho
Copy link
Contributor Author

@danthe3rd I took the easiest path and wrote a python script that generates the version. I didn't want to hack around with bash.

https://test.pypi.org/project/formers/0.0.16rc385/

Copy link
Contributor

@danthe3rd danthe3rd left a comment

Choose a reason for hiding this comment

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

That's perfect - thanks a lot! Let's get this merged once we have greenlights from CI :)

@danthe3rd
Copy link
Contributor

Let's merge :)
Thanks a lot for the contribution @AbdBarho ! let's see if the credentials are correctly configured now

@danthe3rd danthe3rd merged commit 062602c into facebookresearch:main Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants