facebookresearch / theseus

A library for differentiable nonlinear optimization
MIT License
1.74k stars 124 forks source link

Update build_wheel.sh to properly use created conda environment #561

Closed JingyuQian closed 1 year ago

JingyuQian commented 1 year ago

Motivation and Context

This PR tries to fix a bug in build_wheel.sh that causes the theseus conda environment not to be properly used during the build stage

How Has This Been Tested

This is a small fix. Use which pip and which python3 during the build stage to verify. These executables should have /opt/conda/envs/theseus/bin as a path prefix.

Types of changes

Checklist

JingyuQian commented 1 year ago

It looks like one of the ci tests failed. The local build is OK so I'm not sure why.

mhmukadam commented 1 year ago

It looks like one of the ci tests failed. The local build is OK so I'm not sure why.

Thanks for pointing out the bug and for making a PR with the fix. The CI issue is happening on other PRs as well and @luisenp is looking into it.

luisenp commented 1 year ago

The wheel test always fails on external PRs because it's written to tell the build script to use the latest commit as the target to compile, but the commit doesn't exist in our repo (it's part of the fork where the PR comes from).

Usually we can ignore, but since this is modifying the build wheel script, I'm also going to test this locally.

Thanks for the contribution @JingyuQian!