Skip to content

clean up the build a bit. We no longer need the separate build_libtorch entrypoint #9836

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.

Sign up for GitHub

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

Jump to bottom
Closed
anderspapitto wants to merge 1 commit into from
Closed

clean up the build a bit. We no longer need the separate build_libtorch entrypoint #9836

anderspapitto wants to merge 1 commit into from

Conversation

@anderspapitto
Copy link
Contributor

@anderspapitto anderspapitto commented Jul 25, 2018 •
edited
Loading

the new entrypoint is ./tools/build_pytorch_libs.sh caffe2

this will also speed up CI builds a bit, since we will no longer be compiling all of libtorch twice

@ezyang
Copy link
Contributor

ezyang commented Jul 25, 2018

@pytorchbot retest this please

@anderspapitto anderspapitto force-pushed the cleanup-build branch 4 times, most recently from 8e72247 to bab364c Compare July 26, 2018 21:49
@anderspapitto anderspapitto changed the title [WIP/experimental] clean up the build a bit. We no longer need the separate [WIP/experimental] clean up the build a bit. We no longer need the separate build_libtorch entrypoint Jul 26, 2018
@anderspapitto anderspapitto requested a review from ebetica July 26, 2018 21:53
@anderspapitto anderspapitto force-pushed the cleanup-build branch 9 times, most recently from 4b81b7c to 07e5294 Compare July 30, 2018 21:25
@anderspapitto anderspapitto changed the title [WIP/experimental] clean up the build a bit. We no longer need the separate build_libtorch entrypoint [WIP] clean up the build a bit. We no longer need the separate build_libtorch entrypoint Jul 30, 2018
@anderspapitto anderspapitto force-pushed the cleanup-build branch 2 times, most recently from a07b8f6 to c6f4ca3 Compare July 31, 2018 19:16
@anderspapitto anderspapitto changed the title [WIP] clean up the build a bit. We no longer need the separate build_libtorch entrypoint clean up the build a bit. We no longer need the separate build_libtorch entrypoint Aug 1, 2018
goldsborough
goldsborough reviewed Aug 1, 2018
View reviewed changes
Copy link
Contributor

@goldsborough goldsborough left a comment

Choose a reason for hiding this comment

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

Mostly LGTM, just a lot of questions of how my old config will translate to the brave new world

.jenkins/pytorch/build.sh
popd
fi

# Test no-Python build

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

.jenkins/pytorch/macos-build.sh
# this is a bit hacky, but not too bad. Bundle the test binaries into
# the installation directory, so they can catch a free ride on the 7z
# train.
mkdir -p ${PYTORCH_ENV_DIR}/miniconda3/lib/python3.6/site-packages/torch/test_binaries/build

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

tools/cpp_build/build_caffe2.sh

cmake -DUSE_CUDA:BOOL=$USE_CUDA \
-DBUILD_TORCH=ON \
-DUSE_OPENMP:BOOL=${USE_OPENMP:ON} \

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

tools/cpp_build/build_common.sh
fi

# Code is developed a lot more than released, so default to Debug.
BUILD_TYPE=${BUILD_TYPE:-Debug}

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

tools/cpp_build/build_common.sh
# Build with Ninja if available. It has much cleaner output.
GENERATE="Unix Makefiles"
MAKE=make
if [ -x "$(command -v ninja)" ]; then

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

tools/cpp_build/build_common.sh
# Try to build with as many threads as we have cores, default to 4 if the
# command fails.
set +e
if [ -n "$MAX_JOBS" ]; then # Use MAX_JOBS if it is set

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

ezyang
ezyang reviewed Aug 2, 2018
View reviewed changes
.jenkins/pytorch/build.sh

# Add the test binaries so that they won't be git clean'ed away
git add -f build/bin
git add -f build/bin build/lib

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

@goldsborough
Copy link
Contributor

goldsborough commented Aug 6, 2018

What is the status here? Can we make this happen soon?

@anderspapitto
Copy link
Contributor Author

anderspapitto commented Aug 6, 2018

@goldsborough if you want to accept I'm ready to push now. But in the meantime i'm looking at some of the cleanup comments

goldsborough
goldsborough approved these changes Aug 6, 2018
View reviewed changes
Copy link
Contributor

@goldsborough goldsborough left a comment

Choose a reason for hiding this comment

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

@ezyang still had some questions, then he can give you codeowner approval

facebook-github-bot
facebook-github-bot reviewed Aug 6, 2018
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

anderspapitto is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@anderspapitto anderspapitto deleted the cleanup-build branch August 6, 2018 21:06
@goldsborough
Copy link
Contributor

goldsborough commented Aug 6, 2018

I kind of expected you to wait for @ezyang's approval

ezyang added a commit to ezyang/pytorch that referenced this pull request Aug 7, 2018
@ezyang
Revert "clean up the build a bit. We no longer need the separate buil…
6b677d1 
…d_libtorch entrypoint (pytorch#9836)"

This reverts commit 62e23a1.
This was referenced Aug 7, 2018
Closed
Closed
facebook-github-bot pushed a commit that referenced this pull request Aug 7, 2018
@ezyang @facebook-github-bot
Revert "clean up the build a bit. We no longer need the separate buil… (
1a797ec 
#10285)

Summary:
…d_libtorch entrypoint (#9836)"

This reverts commit 62e23a1.
Pull Request resolved: #10285

Differential Revision: D9193107

Pulled By: ezyang

fbshipit-source-id: de96dce12fdf74410413ae18feee5caf0bed0025
goodlux pushed a commit to goodlux/pytorch that referenced this pull request Aug 15, 2018
@anderspapitto
clean up the build a bit. We no longer need the separate build_libtor…
9603138 
…ch entrypoint (pytorch#9836)

Summary:
the new entrypoint is `./tools/build_pytorch_libs.sh caffe2`

this will also speed up CI builds a bit, since we will no longer be compiling all of libtorch twice
Pull Request resolved: pytorch#9836

Differential Revision: D9182634

Pulled By: anderspapitto

fbshipit-source-id: 0b9a20ab04f5df2d5c4e7777e4dc468ab25b9ce2
goodlux pushed a commit to goodlux/pytorch that referenced this pull request Aug 15, 2018
@ezyang
Revert "clean up the build a bit. We no longer need the separate buil… (
421d889 
pytorch#10285)

Summary:
…d_libtorch entrypoint (pytorch#9836)"

This reverts commit 62e23a1.
Pull Request resolved: pytorch#10285

Differential Revision: D9193107

Pulled By: ezyang

fbshipit-source-id: de96dce12fdf74410413ae18feee5caf0bed0025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ezyang ezyang ezyang left review comments

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@apaszke apaszke Awaiting requested review from apaszke

@colesbury colesbury Awaiting requested review from colesbury

@gchanan gchanan Awaiting requested review from gchanan

@soumith soumith Awaiting requested review from soumith

@zdevito zdevito Awaiting requested review from zdevito

@ebetica ebetica Awaiting requested review from ebetica

+1 more reviewer

@goldsborough goldsborough goldsborough approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

open source

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@anderspapitto @ezyang @goldsborough @facebook-github-bot