Skip to content

Conversation

wconstab
Copy link
Contributor

Summary: Original commit changeset: 1c7133627da2

Differential Revision: D26077905

…on interpreter"

Summary: Original commit changeset: 1c7133627da2

Differential Revision: D26077905

fbshipit-source-id: 2d0891b5e2adae2a293cb4ce66ffce54b8a953a7
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jan 26, 2021

💊 CI failures summary and remediations

As of commit c1b20d8 (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D26077905

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in dc2a44c.

@mruberry
Copy link
Collaborator

Sorry @wconstab, unlanding because this appears to have broken pytorch_libtorch_linux_xenial_cuda10_2_cudnn7_py3_gcc7_build. See https://app.circleci.com/pipelines/github/pytorch/pytorch/265512/workflows/196c821f-52c5-40f3-adc1-708ec3a7b810/jobs/10458479. Relevant snippet:

Jan 28 01:04:49 CMake Error at torch/csrc/deploy/interpreter/CMakeLists.txt:79 (target_include_directories):
Jan 28 01:04:49   Cannot specify include directories for target "torch_python_obj" which is
Jan 28 01:04:49   not built by this project.

I don't think this build is run on the Github CI. You can trigger all builds to run in the Github CI by starting your branch name with ci-all/, see https://github.com/pytorch/pytorch/wiki/Developer-FAQ#my-pr-caused-a-build-to-fail-but-its-not-usually-run-in-the-github-ci-how-do-i-test-my-pr-against-every-build.

@facebook-github-bot
Copy link
Contributor

This pull request has been reverted by 12a434a.

@wconstab
Copy link
Contributor Author

Thanks for the pointer @mruberry!

Hmm, I wonder if you can help me with this- I have made a change to build.sh to filter more specifically to only build/use USE_DEPLOY on the build variant i intended. (I was not aware of the libtorch- variants and don't think I want USE_DEPLOY enabled for those anyway).

Since my diff has fbcode changes too, I'm not sure how to push the changes to an oss branch with ci-all in the name.

My build has already passed all the OSS CI builds that run on PRs, including the one where I enable and run/test it, I just want to make sure my codepath doesn't activate on any other flavors. Could you help me take a look at this patch to see if it should be enough?
.jenkins/pytorch/build.sh, line23:

-if [[ "$BUILD_ENVIRONMENT" == linux-xenial-cuda10.2-cudnn7-py3-gcc7 ]]; then
+if [[ "$BUILD_ENVIRONMENT" == pytorch-linux-xenial-cuda10.2-cudnn7-py3-gcc7* ]]; then

@mruberry
Copy link
Collaborator

@wconstab I'd love to help but I'm not our build expert. @malfet?

@malfet
Copy link
Contributor

malfet commented Jan 28, 2021

@wconstab one can always re-run the failed build with ssh to see what is going on.
Very likely, that you guard add_library(torch_python_obj) under some sort of if(USE_DEPLOY), but forgot to do the same for target_include_directories in torch/csrc/deploy/interpreter/CMakeLists.txt
Why do you even want to include this folder if USE_DEPLOY is false?

Also, please note that pytorch_libtorch targets are unique in a sense that they only build libtorch.so, but not the libtorch_python.so

@wconstab
Copy link
Contributor Author

@malfet I do the following already in the root CMakeLists:

# ---[ Torch Deploy
if(USE_DEPLOY)
  add_subdirectory(torch/csrc/deploy)
endif()

However, the reason that didn't save me is that I enabled 'USE_DEPLOY' on the pytorch-libtorch build flavor by mistake, and as you pointed out that build does not build torch_python. I think, my change to build.sh should be good.

-if [[ "$BUILD_ENVIRONMENT" == linux-xenial-cuda10.2-cudnn7-py3-gcc7 ]]; then
+if [[ "$BUILD_ENVIRONMENT" == pytorch-linux-xenial-cuda10.2-cudnn7-py3-gcc7* ]]; then

I've submitted the ci-all PR so we'll see.
#51266

@malfet
Copy link
Contributor

malfet commented Jan 28, 2021

@wconstab To make CMake resilient against conflicting options, please consider making following change to CMakeLists::

if(USE_DEPLOY AND BUILD_PYTHON)
  add_subdirectory(torch/csrc/deploy)
endif()

and/or add

cmake_dependent_option(
    USE_DEPLOY "Build embedded torch interpreter" OFF
    "BUILD_PYTHON" OFF)

after

option(BUILD_PYTHON "Build Python binaries" ON)

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