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 libjpeg-turbo dependency for torchvision #20437

Conversation

smoors
Copy link
Contributor

@smoors smoors commented Apr 29, 2024

(created using eb --new-pr)

@boegel
Copy link
Member

boegel commented May 7, 2024

@smoors Isn't this also relevant for more recent versions of torchvision?

@boegel boegel added the bug fix label May 7, 2024
@boegel boegel added this to the release after 4.9.1 milestone May 7, 2024
@smoors
Copy link
Contributor Author

smoors commented May 7, 2024

@smoors Isn't this also relevant for more recent versions of torchvision?

yes, was planning to, but got side-tracked. is already fixed for

  • PyTorch-bundle-2.1.2-foss-2023a.eb
  • PyTorch-bundle-2.1.2-foss-2023a-CUDA-12.1.1.eb

still to do:

  • torchvision-0.14.1-foss-2022b.eb
  • torchvision-0.16.0-foss-2023a.eb

@smoors
Copy link
Contributor Author

smoors commented May 7, 2024

@boegelbot: please test @ generoso

@boegelbot
Copy link
Collaborator

@smoors: Request for testing this PR well received on login1

PR test command 'EB_PR=20437 EB_ARGS= EB_CONTAINER= EB_REPO=easybuild-easyconfigs /opt/software/slurm/bin/sbatch --job-name test_PR_20437 --ntasks=4 ~/boegelbot/eb_from_pr_upload_generoso.sh' executed!

  • exit code: 0
  • output:
Submitted batch job 13426

Test results coming soon (I hope)...

- notification for comment with ID 2099016221 processed

Message to humans: this is just bookkeeping information for me,
it is of no use to you (unless you think I have a bug, which I don't).

@boegelbot
Copy link
Collaborator

Test report by @boegelbot
SUCCESS
Build succeeded for 4 out of 4 (4 easyconfigs in total)
cns1 - Linux Rocky Linux 8.9, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz (haswell), Python 3.6.8
See https://gist.github.com/boegelbot/d8fe2120c77d48f8e20b956f21f8e10b for a full test report.

@boegel
Copy link
Member

boegel commented May 7, 2024

@smoors Please sync with develop and also update recently merged torchvision easyconfig from #20493

Also, what kind of impact does WITH_CUDA=1 have that is slipping in here?

@smoors
Copy link
Contributor Author

smoors commented May 7, 2024

@smoors Please sync with develop and also update recently merged torchvision easyconfig from #20493

you meant 6 minutes ago ;)

Also, what kind of impact does WITH_CUDA=1 have that is slipping in here?

it doesn't have any effect as we are already setting FORCE_CUDA environment variable in the torchvision easyblock. i just copied this line from PyTorch-bundle-2.1.2-foss-2023a-CUDA-12.1.1.eb.

note that you slipped in WITH_CUDA=0 in #20382, which also doesn't have any effect, so please stay kind :)

@smoors
Copy link
Contributor Author

smoors commented May 7, 2024

on second thought, i think it's better to fix this in the easyblock

update: easybuilders/easybuild-easyblocks#3322

@boegel
Copy link
Member

boegel commented May 7, 2024

note that you slipped in WITH_CUDA=0 in #20382, which also doesn't have any effect, so please stay kind :)

It was a genuine question, I wasn't pretending I'm all-knowing here... :)

('PyTorch', '1.12.0', '-CUDA-%(cudaver)s'),
]

preinstallopts = 'WITH_CUDA=1 TORCHVISION_INCLUDE="$EBROOTLIBJPEGMINTURBO/include:$TORCHVISION_INCLUDE"'
Copy link
Member

Choose a reason for hiding this comment

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

If we're going forward with easybuilders/easybuild-easyblocks#3322 (which makes sense), we should clean this up accordingly...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think we can just close this PR, as libjpeg-turbo is already a dep of Pillow-SIMD?
once the easyblock is merged i'll remove the TORCHVISION_INCLUDE from the easyconfigs that have it set

@smoors
Copy link
Contributor Author

smoors commented May 9, 2024

closing in favor of easybuilders/easybuild-easyblocks#3322

@smoors smoors closed this May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants