Skip to content

Include AT_PARALLEL_OPENMP/AT_PARALLEL_NATIVE/AT_PARALLEL_NATIVE_TBB to ATen/Config.h#39612

Closed
pbelevich wants to merge 5 commits intogh/pbelevich/128/basefrom
gh/pbelevich/128/head
Closed

Include AT_PARALLEL_OPENMP/AT_PARALLEL_NATIVE/AT_PARALLEL_NATIVE_TBB to ATen/Config.h#39612
pbelevich wants to merge 5 commits intogh/pbelevich/128/basefrom
gh/pbelevich/128/head

Conversation

@pbelevich
Copy link
Contributor

@pbelevich pbelevich commented Jun 5, 2020 •
edited
Loading

Fixes #39471

Manually tested with https://github.com/pytorch/csprng

Stack from ghstack:

Differential Revision: D21960728

@pbelevich pbelevich requested review from ezyang and malfet June 5, 2020 23:45
@dr-ci
Copy link

dr-ci bot commented Jun 6, 2020 •
edited
Loading

💊 CI failures summary and remediations

As of commit 981790c (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 on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 9 times.

@pbelevich
Update on "Include AT_PARALLEL_OPENMP/AT_PARALLEL_NATIVE/AT_PARALLEL_…
8d04c86 
…NATIVE_TBB to ATen/Config.h"

Fixes #39471




[ghstack-poisoned]
pbelevich added a commit that referenced this pull request Jun 6, 2020
@pbelevich
Include AT_PARALLEL_OPENMP/AT_PARALLEL_NATIVE/AT_PARALLEL_NATIVE_TBB …
772647d 
…to ATen/Config.h

ghstack-source-id: 62f63ec
Pull Request resolved: #39612
ezyang
ezyang reviewed Jun 7, 2020
View reviewed changes
aten/src/ATen/Config.h.in Outdated
#define AT_NNPACK_ENABLED() @AT_NNPACK_ENABLED@
#define CAFFE2_STATIC_LINK_CUDA() @CAFFE2_STATIC_LINK_CUDA_INT@
#define AT_BUILD_WITH_BLAS() @USE_BLAS@
#ifndef AT_PARALLEL_OPENMP
Copy link
Contributor

@ezyang ezyang Jun 7, 2020

Choose a reason for hiding this comment

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

Why the ifndef?

Copy link
Contributor Author

@pbelevich pbelevich Jun 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

There are files that include Config.h(maybe transitively) and files that don't. For all of them AT_PARALLEL_OPENMP is defined from command line and for those that include Config.h it's redefined there. In the second case warning about redefinition is shown and compilation fails until I guard it with #ifndef

Copy link
Contributor

@ezyang ezyang Jun 8, 2020

Choose a reason for hiding this comment

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

It seems better to just stop passing AT_PARALLEL_OPENMP from command line in this case...

Copy link
Contributor Author

@pbelevich pbelevich Jun 8, 2020

Choose a reason for hiding this comment

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

@ezyang addressed

@pbelevich pbelevich requested a review from ezyang June 7, 2020 23:38
@pbelevich
Update on "Include AT_PARALLEL_OPENMP/AT_PARALLEL_NATIVE/AT_PARALLEL_…
311d9af 
…NATIVE_TBB to ATen/Config.h"

Fixes #39471

Manually tested with https://github.com/pytorch/csprng




[ghstack-poisoned]
@malfet
Copy link
Contributor

malfet commented Jun 9, 2020

You'll need to add same defines to BUILD.bazel

@pbelevich
Update on "Include AT_PARALLEL_OPENMP/AT_PARALLEL_NATIVE/AT_PARALLEL_…
f571c35 
…NATIVE_TBB to ATen/Config.h"

Fixes #39471

Manually tested with https://github.com/pytorch/csprng




[ghstack-poisoned]
@pbelevich
Copy link
Contributor Author

pbelevich commented Jun 9, 2020

@malfet yup, just did it, thanks!

@pbelevich
Update on "Include AT_PARALLEL_OPENMP/AT_PARALLEL_NATIVE/AT_PARALLEL_…
981790c 
…NATIVE_TBB to ATen/Config.h"

Fixes #39471

Manually tested with https://github.com/pytorch/csprng




[ghstack-poisoned]
pbelevich added a commit that referenced this pull request Jun 9, 2020
@pbelevich
Include AT_PARALLEL_OPENMP/AT_PARALLEL_NATIVE/AT_PARALLEL_NATIVE_TBB …
cf8536e 
…to ATen/Config.h

ghstack-source-id: 350cf5c
Pull Request resolved: #39612
ezyang
ezyang approved these changes Jun 9, 2020
View reviewed changes
Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

Thank you!

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jun 11, 2020

@pbelevich merged this pull request in 4c4b991.

@zou3519
Copy link
Contributor

zou3519 commented Jun 11, 2020

@pbelevich
Sometime after this PR went in there has been some flakiness in the TorchVision build that errors out with /opt/conda/lib/python3.6/site-packages/torch/include/ATen/ParallelNativeTBB.h:12:21: fatal error: tbb/tbb.h: No such file or directory (see https://circleci.com/gh/pytorch/pytorch/5790606?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link and https://circleci.com/gh/pytorch/pytorch/5787534?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link)

This PR is the only recent one I can see that has modified anything related to tbb, so in an abundance of caution I am reverting it.

@pbelevich pbelevich mentioned this pull request Jun 12, 2020
Closed
@facebook-github-bot facebook-github-bot deleted the gh/pbelevich/128/head branch June 14, 2020 14:16
@pbelevich pbelevich mentioned this pull request Jun 15, 2020
Closed
pbelevich added a commit that referenced this pull request Jun 15, 2020
@pbelevich
Update on "[Yet Another Reland] Include AT_PARALLEL_OPENMP/AT_PARALLE…
8806993 
…L_NATIVE/AT_PARALLEL_NATIVE_TBB to ATen/Config.h"

Fixes #39471

Reland of #39612 and #39881


Differential Revision: [D22050656](https://our.internmc.facebook.com/intern/diff/D22050656)

[ghstack-poisoned]
pbelevich added a commit that referenced this pull request Jun 16, 2020
@pbelevich
Update on "[Yet Another Reland] Include AT_PARALLEL_OPENMP/AT_PARALLE…
616629a 
…L_NATIVE/AT_PARALLEL_NATIVE_TBB to ATen/Config.h"

Fixes #39471

Reland of #39612 and #39881


Differential Revision: [D22050656](https://our.internmc.facebook.com/intern/diff/D22050656)

[ghstack-poisoned]
facebook-github-bot pushed a commit that referenced this pull request Jun 16, 2020
@pbelevich @facebook-github-bot
[Yet Another Reland] Include AT_PARALLEL_OPENMP/AT_PARALLEL_NATIVE/AT…
d4faf14 
…_PARALLEL_NATIVE_TBB to ATen/Config.h (#40045)

Summary:
Pull Request resolved: #40045

Fixes #39471

Reland of #39612 and #39881

Test Plan: Imported from OSS

Differential Revision: D22050656

Pulled By: pbelevich

fbshipit-source-id: 274bc0733c37ef6c87deb3344bb49ca9107e257b
@pbelevich pbelevich mentioned this pull request Jun 16, 2020
Closed
pbelevich added a commit that referenced this pull request Jun 16, 2020
@pbelevich
Update on "[Reland #3] Include AT_PARALLEL_OPENMP/AT_PARALLEL_NATIVE/…
a15d83d 
…AT_PARALLEL_NATIVE_TBB to ATen/Config.h"

Fixes #39471

Reland of #39612 #39881 and #40045


Differential Revision: [D22076711](https://our.internmc.facebook.com/intern/diff/D22076711)

[ghstack-poisoned]
pbelevich added a commit that referenced this pull request Jun 16, 2020
@pbelevich
Update on "[Reland #3] Include AT_PARALLEL_OPENMP/AT_PARALLEL_NATIVE/…
c9f0a84 
…AT_PARALLEL_NATIVE_TBB to ATen/Config.h"

Fixes #39471

Reland of #39612 #39881 and #40045


Differential Revision: [D22076711](https://our.internmc.facebook.com/intern/diff/D22076711)

[ghstack-poisoned]
pbelevich added a commit that referenced this pull request Jun 16, 2020
@pbelevich
Update on "[Reland #3] Include AT_PARALLEL_OPENMP/AT_PARALLEL_NATIVE/…
e064c13 
…AT_PARALLEL_NATIVE_TBB to ATen/Config.h"

Fixes #39471

Reland of #39612 #39881 and #40045


Differential Revision: [D22076711](https://our.internmc.facebook.com/intern/diff/D22076711)

[ghstack-poisoned]
pbelevich added a commit that referenced this pull request Jun 16, 2020
@pbelevich
Update on "[Reland #3] Include AT_PARALLEL_OPENMP/AT_PARALLEL_NATIVE/…
7bd9b9e 
…AT_PARALLEL_NATIVE_TBB to ATen/Config.h"

Fixes #39471

Reland of #39612 #39881 and #40045


Differential Revision: [D22076711](https://our.internmc.facebook.com/intern/diff/D22076711)

[ghstack-poisoned]
@pbelevich pbelevich mentioned this pull request Jun 18, 2020
Closed
facebook-github-bot pushed a commit that referenced this pull request Jun 19, 2020
@pbelevich @facebook-github-bot
[Reland #4] Include AT_PARALLEL_OPENMP/AT_PARALLEL_NATIVE/AT_PARALLEL…
ccea372 
…_NATIVE_TBB to ATen/Config.h (#40211)

Summary:
Fixes #39471

Reland of #39612 #39881 #40045 #40122

Proof: [green TBB test](https://app.circleci.com/pipelines/github/pytorch/pytorch/182769/workflows/ae9f4f7a-791a-49df-9625-e2f0a51e70e7/jobs/5910591/steps)
Pull Request resolved: #40211

Reviewed By: malfet

Differential Revision: D22128537

Pulled By: pbelevich

fbshipit-source-id: 98c589405daafc2c81f76e1d5c1aef5e57065351
xwang233 pushed a commit to xwang233/pytorch that referenced this pull request Jun 20, 2020
@pbelevich @xwang233
[Yet Another Reland] Include AT_PARALLEL_OPENMP/AT_PARALLEL_NATIVE/AT…
99c8173 
…_PARALLEL_NATIVE_TBB to ATen/Config.h (pytorch#40045)

Summary:
Pull Request resolved: pytorch#40045

Fixes pytorch#39471

Reland of pytorch#39612 and pytorch#39881

Test Plan: Imported from OSS

Differential Revision: D22050656

Pulled By: pbelevich

fbshipit-source-id: 274bc0733c37ef6c87deb3344bb49ca9107e257b
xwang233 pushed a commit to xwang233/pytorch that referenced this pull request Jun 20, 2020
@pbelevich @xwang233
[Reland pytorch#4] Include AT_PARALLEL_OPENMP/AT_PARALLEL_NATIVE/AT_P…
501c44b 
…ARALLEL_NATIVE_TBB to ATen/Config.h (pytorch#40211)

Summary:
Fixes pytorch#39471

Reland of pytorch#39612 pytorch#39881 pytorch#40045 pytorch#40122

Proof: [green TBB test](https://app.circleci.com/pipelines/github/pytorch/pytorch/182769/workflows/ae9f4f7a-791a-49df-9625-e2f0a51e70e7/jobs/5910591/steps)
Pull Request resolved: pytorch#40211

Reviewed By: malfet

Differential Revision: D22128537

Pulled By: pbelevich

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

Reviewers

@ezyang ezyang ezyang approved these changes

@malfet malfet Awaiting requested review from malfet

Assignees

No one assigned

Labels

Merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@pbelevich @malfet @facebook-github-bot @zou3519 @ezyang @mruberry