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

BUG: Update dep pinnings #80

Merged
merged 10 commits into from
Nov 13, 2023
Merged

Conversation

carterbox
Copy link
Member

@carterbox carterbox commented Nov 8, 2023

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

Closes #79
Partial #22

The purpose of this PR is to update the pytorch pinnings to match upstream according to their stated compatability matrix and according to their runtime checks.

I did this by

  1. Manually setting the pytorch version instead of trying to capture the channel-wide pinnings, so that this package is always uses the version of pytorch declared by upstream as compatible.
  2. Including the CUDA version in the build string pinning because torchvision checks that the pytorch library was compiled with the same minor version of CUDA.

As a bonus, I also:

  1. Added site packages to the rpath to supress linker warnings
  2. Added missing pip args --no-deps --no-build-isolation which are not automatically added by us when pip is called from a script in the outputs section instead of from a build script in the top-level package.
  3. Skipped a test broken by a new version of pillow

@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@carterbox
Copy link
Member Author

Not sure why we're trying to run almost all of the unit tests, but these tests are failing on test_draw_boxes.

Upstream has identified the issue as something caused by a change with pillow 10.1.0, so it is unrelated to this PR. They fixed the test by updating the reference image in a later release, so it's fine that this test fails. I think the best approach would be to skip that test because it doesn't indicate a problem for downstream users.

@carterbox
Copy link
Member Author

@conda-forge-admin, please rerender

@h-vetinari
Copy link
Member

Not sure why we're trying to run almost all of the unit tests

Because just import tests or small test scripts are woefully inadequate for testing whether a given package is fully functional. I think it should be our standard everywhere (modulo timeout/resource issues).

PS. Thanks for tackling this! 🙏

@hmaarrfk
Copy link
Contributor

Because just import tests or small test scripts are woefully inadequate for testing whether a given package is fully functional. I think it should be our standard everywhere (modulo timeout/resource issues).

yeah... but it adds 10MB to the package..... so its less than idea IMO.

@hmaarrfk hmaarrfk merged commit 15ab519 into conda-forge:main Nov 13, 2023
22 checks passed
@hmaarrfk
Copy link
Contributor

Thank you!

@h-vetinari
Copy link
Member

but it adds 10MB to the package..... so its less than idea IMO.

So let's split off the tests into a separate output - best of both worlds (except that you don't like multi output recipes 😜)

@hmaarrfk
Copy link
Contributor

lol, that is a true statement ....

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

Successfully merging this pull request may close these issues.

CUDA minor version mismatch error between pytorch and torchvision possible
3 participants