facebookresearch / theseus

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

Potential bug in the build script #554

Closed JingyuQian closed 1 year ago

JingyuQian commented 1 year ago

πŸ› Bug

The build script does not actually use the created theseus conda environment

Steps to Reproduce

Steps to reproduce the behavior:

in the build_wheel.sh, change the contents of dockerfile from:

# --- Install torch
ENV CUDA_HOME /usr/local/cuda-cpu
RUN pip install "torch==1.13" --extra-index-url https://download.pytorch.org/whl/cpu

to:

# --- Install torch
ENV CUDA_HOME /usr/local/cuda-cpu
RUN which pip && ls /opt/conda/envs/theseus/bin/pip && pip install "torch==1.13" --extra-index-url https://download.pytorch.org/whl/cpu

And run the build script.

Expected behavior

There will be 2 additional lines of output:

/opt/conda/bin/pip
/opt/conda/envs/theseus/bin/pip

Which means the pip used in the building stage is not the one from the "theseus" conda env.

Additional context

The problem lies in this line:

RUN source activate theseus

It does activate the theseus env, but then Docker creates a layer from this and proceeds to the next command and won't use the theseus env from this layer. The 3.10 actually comes from the conda base env. So pip and python3 commands should be modified, such as creating some new environment variables.

Bit of context

Was trying to build theseus with a lower version of PyTorch and python3.9. The wheel builds successfully but keeps tagging python3.10, which lead me to find this problem.

luisenp commented 1 year ago

Hi @JingyuQian, thanks for reporting this! I noticed this behavior a couple weeks ago but didn't look too deeply into what the cause was, this is very helpful!

Would you be interested in submitting a pull request with a fix?

JingyuQian commented 1 year ago

Hi @luisenp , I've submitted a PR to fix this issue. Feel free to verify, thanks!