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

Use libjpeg-turbo #7672

Merged
merged 18 commits into from
Aug 9, 2023
Merged

Use libjpeg-turbo #7672

merged 18 commits into from
Aug 9, 2023

Conversation

NicolasHug
Copy link
Member

@NicolasHug NicolasHug commented Jun 15, 2023

libjpeg-turbo is now available on the pytorch conda channel so we should be able to switch (at least for cuda builds).
Probably superseeds #5951

Addresses #7660

@pytorch-bot
Copy link

pytorch-bot bot commented Jun 15, 2023

@pmeier
Copy link
Collaborator

pmeier commented Jun 19, 2023

libjpeg-turbo is now available on the pytorch conda channel

That is not true and the reason why we are seeing all the errors above. It is available from the pytorch-nightly channel though: https://anaconda.org/pytorch-nightly/libjpeg-turbo

Comment on lines 37 to 38
# 1. `libjpeg-turbo` is only available for macOS and Windows on the `defaults` conda channel. Thus, we need to pull it
# in from the `pytorch-nightly` channel on Linux
Copy link
Collaborator

Choose a reason for hiding this comment

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

Meaning, we can't just do conda install -c pytorch-nightly libjpeg-turbo, since that won't work for Windows and macOS: https://anaconda.org/pytorch-nightly/libjpeg-turbo/files

We could either set the channel based on the OS, or let conda figure it out automatically by proving both channels to the initial env creation.

I've opted for the latter for now. Let's see if CI is happy with it.

@pmeier
Copy link
Collaborator

pmeier commented Jun 19, 2023

We have reverted the fixes I made since libjpeg-turbo should be accessible on the stable pytorch channel soon for all OSs and archs. For now we only have it on pytorch-nightly and only for Linux / x86: https://anaconda.org/pytorch-nightly/libjpeg-turbo/files

Still, note that CI is green for fd39e13 minus a few flaky tests. Meaning, it should work out smoothly.

@NicolasHug NicolasHug marked this pull request as ready for review August 4, 2023 10:26
Copy link
Collaborator

@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.

LGTM if CI is green. Thanks Nicolas!

.github/scripts/setup-env.sh Outdated Show resolved Hide resolved
torchvision/csrc/io/image/cpu/decode_jpeg.cpp Show resolved Hide resolved
Co-authored-by: Philip Meier <github.pmeier@posteo.de>
@NicolasHug
Copy link
Member Author

CC @malfet @atalman , just making sure this is OK with you before merging. TL;DR we now enforce that build jobs (and tests job) are built with libjpeg-turbo. This will prevent compatibility issues as noticed in #7660

Copy link
Contributor

@atalman atalman left a comment

Choose a reason for hiding this comment

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

lgtm, we may need to build libjpeg-turbo for aarch64

@NicolasHug NicolasHug merged commit 1db6907 into pytorch:main Aug 9, 2023
43 of 60 checks passed
@github-actions
Copy link

github-actions bot commented Aug 9, 2023

Hey @NicolasHug!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

@NicolasHug
Copy link
Member Author

(labelling as bug, not strictly the case but I want to make sure we advertize this properly in our release notes)

@NicolasHug NicolasHug mentioned this pull request Aug 10, 2023
facebook-github-bot pushed a commit that referenced this pull request Aug 25, 2023
Summary: Co-authored-by: Philip Meier <github.pmeier@posteo.de>

Reviewed By: matteobettini

Differential Revision: D48642301

fbshipit-source-id: bbf318b1568fdc24796c4e6649d010bdfbd27a44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants