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

Making built wheels available for install #533

Closed
AbdBarho opened this issue Nov 20, 2022 · 52 comments
Closed

Making built wheels available for install #533

AbdBarho opened this issue Nov 20, 2022 · 52 comments

Comments

@AbdBarho
Copy link
Contributor

AbdBarho commented Nov 20, 2022

🚀 Feature

Motivation

After #523 #534, the wheels can be built, but are not available for install anywhere. But users want this #532 #473

Pitch & Alternatives

There a couple of ways that I know of to achieve this:

Upload to pypi

Probably the most obvious solution

  • max file size for pypi is 60MB, or roughly 2-3 CUDA architectures per wheel
    • maybe it makes sense to have per architecture? sounds like an overkill to me
    • one can request more size from the pypi team, I am not sure if this solution scales
  • how would pip find the correct wheel for a given torch & cuda version?
    • probably using an extra pip index just like pytorch does it, but it is not "trivial" since now we need to setup an external service to host index, and in that case:

External hosting

Upload the built wheel to an external storage / hosting service (s3 & co.)

  • no size limits
  • additional costs
  • now it is not about python or wheels anymore, there is infrastructure management & maintenance that needs to taken care of.
  • does not solve the question of: how would pip find the correct wheel for a given torch & cuda version?
    • the pip index still needs to be setup (and updated automatically by every release), extra complexity

Upload to the release page on github

Similar to the above, but using github

  • max file size is 2GB (which should be enough)
  • no costs
  • still does not answer the pip question
  • download times are slower than others, but this should not be a problem

maybe it would be easiest to dump all wheels to github releases and then the users would have to find the version they want and install it.

pip install https://github.com/..../xformers-1.14.0+torch1.12.1cu116-cp39-cp39-linux_x86_64.whl

Although this would make it really annoying for libraries that use xformers and want to build for different python /cuda versions, you cannot just add xformers to your requirements.txt.

Additional info & Resources

https://peps.python.org/pep-0503/

https://packaging.python.org/en/latest/guides/hosting-your-own-index/

I am willing to contribute if I know how.

@danthe3rd
Copy link
Contributor

  • max file size for pypi is 60MB

Apparently it's now 100MB. I've requested an increase to 300MB which should give us some margin
pypi/support#2421

how would pip find the correct wheel for a given torch & cuda version?

That's the reason why we are currently using conda, which handles this sort of dependencies.
I see a few options:
(1) pip install manually with the full path from s3
(2) pip install xformers downloads a source distribution. At setup, we retrieve the pytorch/cuda/xformers/python/os version, and download the corresponding binary from s3 if available. Otherwise we build from source
(3) Pin a pytorch version in the requirements, and hope we have compatibility between different cuda versions (I've frequently mismatched 11.6/11.7 without issues..)

In any case, a follow-up task would be to check at run-time that the pytorch version has not changed

What do you think?

@AbdBarho
Copy link
Contributor Author

Overall I think its a good plan, some comments:

(2) pip install xformers downloads a source distribution. At setup, we retrieve the pytorch/cuda/xformers/python/os version, and download the corresponding binary from s3 if available. Otherwise we build from source

Could this in anyway lead to endless loops? I don't know how, but my spider sense is tingling.

Also, the order of installs here matters right? it won't not be possible to

pip install torch xformers --extra-index-url https://download.pytorch.org/whl/cu116

(3) Pin a pytorch version in the requirements, and hope we have compatibility between different cuda versions (I've frequently mismatched 11.6/11.7 without issues..)

same experience

Perhaps maybe a bit more concretely on the plan forward:

  1. Wait for pypi team and add pip upload to the pipeline
    • what if we don't get the storage increase?
  2. Upload wheels to S3
    • how is the lifetime of the bucket managed? in this repo or somewhere else?
    • is this point mutually exclusive with point 1?

@danthe3rd
Copy link
Contributor

Also, the order of installs here matters right? it won't not be possible to

I think we can have pytorch an install dependency or something like that.
The thing is: we already import & use torch in the setup.py script (https://github.com/facebookresearch/xformers/blob/main/setup.py#L20)

Wait for pypi team and add pip upload to the pipeline

This does not solve the issue with multiple versions of pytorch/cuda (unless we ignore cuda mismatch errors, and add a pinned version of pytorch - latest? - as a dependency)

@danthe3rd
Copy link
Contributor

Could this in anyway lead to endless loops? I don't know how, but my spider sense is tingling.

Oh I meant just download the built binary files (not the entire package). Like _C.so for linux

@AbdBarho
Copy link
Contributor Author

@danthe3rd now that we have pypi storage, what is the next step? is there something I can help with? do we still need to worry about the pip index? it is just an html page after all but still.

@danthe3rd
Copy link
Contributor

I'm trying to figure out internally how we can store that on s3 - it's thanksgiving in the US so hopefully we will have more news to share next week.

@AbdBarho
Copy link
Contributor Author

@danthe3rd happy thanksgiving! ping me when you have any updates.

@danthe3rd
Copy link
Contributor

danthe3rd commented Nov 29, 2022

It looks a bit more complicated than I expected - for now I think we could upload to pypi directly.
I think a first step would be to upload files corresponding to the latest pytorch version on pypi (eg pytorch 1.13 with cu116 at the moment) for win/linux. It looks like we need a few things:
(1) We need to build on "manylinux" rather than ubuntu (at least for linux) - or we might just hack it for now
(2) We should also change the version for the builds we upload, as pypi won't support local prefixes (eg +git{HASH}.d{DATE} that we use currently). We could add a ".dev" suffix with the number of commits in the branch, like we do for conda builds
(3) We would also need a source release I think

What do you think?
I can setup the secrets for twine once this is working :)

@AbdBarho
Copy link
Contributor Author

for 1) we are already building on manylinux

container: ${{ contains(matrix.os, 'ubuntu') && 'quay.io/pypa/manylinux2014_x86_64' || null }}

but the generated wheel has the suffix linux_x64_86, maybe we can cheese it and sed to manylinux, I used the 2014 version for python version support https://github.com/pypa/manylinux#manylinux

for 2) I always thought this was handled by pip automatically if the last commit is tagged, this requires some testing.

And for 3) an additional source build should be easy, since we don't need cuda / torch / compilation, unless I am missing something.

I have never uploaded to pip before, so I will have to play around first to get a feeling for it....

@danthe3rd
Copy link
Contributor

danthe3rd commented Nov 29, 2022

I've just added a secret "PYPI_TOKEN" (starting with pypi-) that should have access to the xformers pypi project.
We can try like this and iterate. The token should be configured this way and then twine can be used to upload packages

(Maybe it's easier if you create your own pypi account/repo to test ...)

@AbdBarho
Copy link
Contributor Author

@danthe3rd yeah, I will start with a dummy repo and https://test.pypi.org/, also check the versioning thing, hopefully over the weekend.

@danthe3rd
Copy link
Contributor

Sorry for the lack of updates recently, with the pytorch conference it took some time to get the information I needed.

So apparently there is this way of building libraries that pytorch provides called NOVA (see the doc). Once we have our setup in this format, we can just add the secrets and it should work.
We will be able to upload the builds to pytorch nightly just by adding the secrets (both conda+pip), and I'm trying to see how we can get that added with pytorch's main packages (for stable releases).

Would you be interested in getting us started with NOVA? It should take care of all the specificities of each platform for us in theory

@AbdBarho
Copy link
Contributor Author

AbdBarho commented Dec 8, 2022

@danthe3rd I can give it a look, it seems like it simplifies (or maybe just unifies) the build workflows across all pytorch related repos.

From a first glance, it seems that it still does not handle the case of uploading to pypi, but only to pytorch's s3. would that be enough? do we will still need a separate step with twine for pypi? what is the plan here?

regarding your point of getting it to stable, maybe it is just an env variable?

https://github.com/pytorch/test-infra/blob/eac24072f2d73c2147846e9731548c849cd8f42e/tools/scripts/generate_binary_build_matrix.py#L410-L411

https://github.com/pytorch/test-infra/blob/eac24072f2d73c2147846e9731548c849cd8f42e/.github/workflows/build_wheels_linux.yml#L175

@danthe3rd
Copy link
Contributor

That is right. @fmassa how is pypi upload handled in torchvision?
I assume we should be able to rely on NOVA to create the builds and then upload however we want.

Otherwise, we can also upload manually with twine if you want to give it a shot with a test repo.

@AbdBarho
Copy link
Contributor Author

AbdBarho commented Dec 9, 2022

@danthe3rd I managed to get NOVA running:

https://github.com/AbdBarho/xformers-wheels/actions/runs/3655705207

I will clean up the code and create a PR.

pypi upload could be added after that from the generated artefacts

@danthe3rd
Copy link
Contributor

Oh wow that was fast :o
Thanks a lot for doing this! If you happen to have any issue with NOVA, let me know and we can have them update their scripts :) (I was wondering if they have the pagefile size increase working for windows build to prevent OOM...)

@danthe3rd
Copy link
Contributor

danthe3rd commented Dec 13, 2022

Otherwise, we can also upload manually with twine if you want to give it a shot with a test repo.

It might be the way forward for the time being, as NOVA does not seem to work for us, and we already have the builds on the CI. What do you think? Do you have time to work on that? I have already setup the PYPI_TOKEN in the repo secrets for github actions

@AbdBarho
Copy link
Contributor Author

AbdBarho commented Dec 13, 2022

@danthe3rd already working on it!

https://github.com/AbdBarho/xformers-wheels/actions/runs/3684497349/jobs/6234302879

with a successful upload:

https://test.pypi.org/project/formers/0.0.15.dev383/

I used the name formers because xformers is already used (duh).

I will clean up the code and create a PR. I have some questions that we can clear up then.

@danthe3rd
Copy link
Contributor

Oh nice! Let's only upload for the latest pytorch stable (1.13 at the moment), with cuda 11.7 for instance.

@FrancescoSaverioZuppichini

Thanks for working on this 🙏

I've downloaded the whl file formers-0.0.15.dev383-cp310-cp310-manylinux2014_x86_64.whl , I am on amd CPU so I guess I have to use an Intel one right?

pip install formers-0.0.15.dev383-cp310-cp310-manylinux2014_x86_64.whl 
ERROR: formers-0.0.15.dev383-cp310-cp310-manylinux2014_x86_64.whl is not a supported wheel on this platform.

@AbdBarho
Copy link
Contributor Author

AbdBarho commented Dec 13, 2022

@danthe3rd That is one question answered, which is the same as what I had in mind:
https://github.com/AbdBarho/xformers-wheels/blob/fde92dc29741be208431c9d1dd0ed365129066c9/.github/workflows/wheels.yml#L148

The second question is regarding the version, do we want to upload dev versions? or only tagged commits? and how should the version string look like?

I saw the conda example, the generated version string is not accepted by pypi, what I have now is just dev + num commits from latest tag.

@AbdBarho
Copy link
Contributor Author

@FrancescoSaverioZuppichini oh oh! can you give me some details about your os? the wheel should (in theory) work on any linux machine, if it doesn't, we have a problem.

@FrancescoSaverioZuppichini

@AbdBarho Sure, from neofetch

image

@danthe3rd
Copy link
Contributor

danthe3rd commented Dec 13, 2022

The second question is regarding the version, do we want to upload dev versions? or only tagged commits? and how should the version string look like?

The optimal solution I see would be:
(1) Upload a "pre-release" for next version at every commit on main - and remove those regularly before we run out of space
(2) Upload a tagged release when we tag a version-like branch
We can start with whatever is easier for you

I saw the conda example, the generated version string is not accepted by pypi, what I have now is just dev + num commits from latest tag.

Yes, pypi won't accept local suffixes. Let's remove them from the version number.

@AbdBarho
Copy link
Contributor Author

AbdBarho commented Dec 13, 2022

@FrancescoSaverioZuppichini I managed to install the wheel on a debian and centos system, the error you have is unexpected.

This wheel is for python 3.10, can you try again with

python3.10 -m pip install ./formers-0.0.15.dev383-cp310-cp310-manylinux2014_x86_64.whl

?

@danthe3rd
Copy link
Contributor

And we just have our first binary wheel uploaded!
https://pypi.org/project/xformers/0.0.16rc377/#files

You can test it with pip install -U --pre xformers

@JacobAsmuth
Copy link

JacobAsmuth commented Dec 15, 2022

Thanks Dan! I really appreciate your hard work!

After installing xformers with

pip install -U --pre xformers

and starting up the stable diffusion 2.0 model (run through the stable-diffusion-ui), I get the following error:

RuntimeError: No such operator xformers::efficient_attention_forward_cutlass - did you forget to build xformers with 'python setup.py develop'?

Full paste here: https://pastebin.com/HZFrgE3M

@AbdBarho
Copy link
Contributor Author

@JacobAsmuth are you running pytorch 1.12.x?

@JacobAsmuth
Copy link

I am not, I'm running pytorch 1.11.0 .

@AbdBarho
Copy link
Contributor Author

Unfortunately, The wheels are built against the latest pytorch 1.13 :/
We have some wheels for 1.12 here, but nothing for 1.11.

Maybe @danthe3rd has any ideas?

@JacobAsmuth
Copy link

JacobAsmuth commented Dec 15, 2022

I'm using 1.11 only because that's what https://github.com/Stability-AI/stablediffusion installs with their default requirments.txt - let me see if it's compatible with later versions.

@danthe3rd
Copy link
Contributor

Did we miss something @AbdBarho ? pip installing XFormers should have updated PyTorch as it's a requirement to have version==1.13...

We definitely need a better error message btw.

@AbdBarho
Copy link
Contributor Author

AbdBarho commented Dec 15, 2022

Not sure, checking the METADATA of the wheel shows that it looks okay:

Metadata-Version: 2.1
Name: xformers
Version: 0.0.16rc379
Summary: XFormers: A collection of composable Transformer building blocks.
Home-page: https://facebookresearch.github.io/xformers/
Author: Facebook AI Research
Author-email: oncall+xformers@xmail.facebook.com
Classifier: Programming Language :: Python :: 3.7
Classifier: Programming Language :: Python :: 3.8
Classifier: Programming Language :: Python :: 3.9
Classifier: Programming Language :: Python :: 3.10
Classifier: License :: OSI Approved :: BSD License
Classifier: Topic :: Scientific/Engineering :: Artificial Intelligence
Classifier: Operating System :: OS Independent
Requires-Python: >=3.7
Description-Content-Type: text/markdown
License-File: LICENSE
Requires-Dist: numpy
Requires-Dist: pyre-extensions (==0.0.23)
Requires-Dist: einops
Requires-Dist: torch (==1.13)

XFormers: A collection of composable Transformer building blocks.XFormers aims at being able to reproduce most architectures in the Transformer-family SOTA,defined as compatible and combined building blocks as opposed to monolithic models

and the same applies to the source distribution.

Edit: I tested both on a fresh install and on an env with torch 1.12, in both cases torch 1.13 is downloaded.

Maybe because it is a pre-release? or maybe its because torch was installed with conda and xformers with pip?

@netw0rkf10w
Copy link

@danthe3rd @AbdBarho Does this work for torch 1.14 (aka 2.0) please?

@debdip
Copy link

debdip commented Dec 15, 2022

It does work on torch 1.13.
I just installed it without any error.

previously I had some error about compilation problem while compile from source.

image

@netw0rkf10w
Copy link

@danthe3rd @AbdBarho Does this work for torch 1.14 (aka 2.0) please?

Would it be possible to relax the torch==1.13 constraint to torch>=1.13 for the wheels?

@danthe3rd
Copy link
Contributor

Would it be possible to relax the torch==1.13 constraint to torch>=1.13 for the wheels?

xFormers works with any version of pytorch if you build yourself, but for the pre-built binary wheels, they are only compatible with the pytorch version they were built for. For now we chose to stick to the latest stable

@netw0rkf10w
Copy link

@danthe3rd Sorry I thought that pip install was building the package. I'll just install from source then. Thanks.

@danthe3rd danthe3rd mentioned this issue Dec 15, 2022
7 tasks
@danthe3rd
Copy link
Contributor

danthe3rd commented Dec 15, 2022

@AbdBarho actually looking at how releases are tagged in pytorch (and previous releases in xformers), we will stick with tags like vA.B.C. Do you think you can update the script for that?
I've also added a todo-list to track release of 0.0.16 in #595 - hopefully this week or next week

@danthe3rd
Copy link
Contributor

danthe3rd commented Dec 15, 2022

Hum it also looks like we have issues with the RC releases now: https://github.com/facebookresearch/xformers/actions/runs/3703118568/jobs/6274182582#step:10:21
Maybe we should use the context for the github action somewhere; https://docs.github.com/en/actions/learn-github-actions/contexts#github-context (github.event ...)

@FrancescoSaverioZuppichini

@AbdBarho it works! 🚀 Thanks a lot ❤️

Tested on python 3.9/3.10

@FrancescoSaverioZuppichini
Copy link

@AbdBarho wait ... you are the hero who made https://github.com/AbdBarho/stable-diffusion-webui-docker sending all the love of this world man! 🤗

@pcuenca
Copy link

pcuenca commented Dec 16, 2022

This is awesome!

We (Hugging Face diffusers team) really want to recommend users to install xFormers, and it'll be much easier when pip wheels are available.

My understanding from #591 is that wheels will be automatically made available via pip starting from version 0.0.16 (for PyTorch 1.13). Is that correct?

@danthe3rd
Copy link
Contributor

Yes that is correct! We will also pin the associated PyTorch version, as only a single pytorch can be supported per pypi version for binary wheels

@pcuenca
Copy link

pcuenca commented Dec 16, 2022

Yes that is correct! We will also pin the associated PyTorch version, as only a single pytorch can be supported per pypi version for binary wheels

Excellent, we'll update our documentation when 0.0.16 is released. Thanks a lot!

@danthe3rd
Copy link
Contributor

danthe3rd commented Jan 12, 2023

@AbdBarho FYI we changed the version for the pip wheels to match the conda versions: 49f72b0

@AbdBarho
Copy link
Contributor Author

AbdBarho commented Feb 1, 2023

Can we close this now? it seems that everything is working fine.

@danthe3rd
Copy link
Contributor

Yes indeed, good point.
Thanks again for all your contributions!

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

No branches or pull requests

7 participants