Skip to content

[caffe2]Export symbols for pybind and other libs after caffe2 rebase #11975

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
gujinghui wants to merge 1 commit into from
Closed

[caffe2]Export symbols for pybind and other libs after caffe2 rebase #11975

gujinghui wants to merge 1 commit into from

Conversation

@gujinghui
Copy link
Collaborator

@gujinghui gujinghui commented Sep 22, 2018

Export symbols for pybind and other libs after caffe2 rebase

@gujinghui gujinghui mentioned this pull request Sep 22, 2018
Closed
@gujinghui
Copy link
Collaborator Author

gujinghui commented Sep 22, 2018

@yinghai pls help review

@yinghai
Copy link
Contributor

yinghai commented Sep 22, 2018

What's the test command to show before and after this PR?

@gujinghui
Copy link
Collaborator Author

gujinghui commented Sep 23, 2018

@yinghai

The two failures above should be not related to ideep code.
In build log, you can see USE_MKL option is OFF, so dose USE_IDEEP.

In recent master branch, the USE_IDEEP is always disabled, that's why there're so many issues...
And I'm trying to fix all of them in #11853, simplify iDEEP options and enable MKL-DNN.
Pls help me.

To enable iDEEP for caffe2 only, please try below config.
cmake -DBLAS=MKL -DUSE_MKL=ON -DUSE_IDEEP=ON -DUSE_MKLML=ON ..

@yinghai
Copy link
Contributor

yinghai commented Sep 24, 2018

Could you rebase on upon recent master?

@gujinghui
Copy link
Collaborator Author

gujinghui commented Sep 25, 2018

@yinghai Done. pls have a try.

@gujinghui
Copy link
Collaborator Author

gujinghui commented Sep 25, 2018

@yinghai
there're still tow failures. It should be not related to this PR.
Pls help confirm.

@ssnl ssnl mentioned this pull request Sep 25, 2018
Closed
@yinghai
Copy link
Contributor

yinghai commented Sep 25, 2018

I think this is suspicious. Master doesn't have any failures.

@ezyang
Copy link
Contributor

ezyang commented Sep 25, 2018

this was briefly broken on master.

@ezyang
Copy link
Contributor

ezyang commented Sep 25, 2018

Did you intend to update the submodule?

@ezyang
Copy link
Contributor

ezyang commented Sep 25, 2018

@pytorchbot retest this please

facebook-github-bot
facebook-github-bot reviewed Sep 25, 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.

yinghai has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

yinghai
yinghai approved these changes Sep 25, 2018
View reviewed changes
Copy link
Contributor

@yinghai yinghai left a comment

Choose a reason for hiding this comment

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

Cool! Looks like things are good. Next step would be unifying the build for MKLDNN.

yinghai
yinghai suggested changes Sep 26, 2018
View reviewed changes
caffe2/ideep/utils/ideep_context.h Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

@gujinghui
Copy link
Collaborator Author

gujinghui commented Sep 26, 2018

@yinghai
rebased to use C10_EXPORT.
pls review.

facebook-github-bot
facebook-github-bot reviewed Sep 26, 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.

yinghai has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@sadatnfs sadatnfs mentioned this pull request Oct 22, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

+1 more reviewer

@yinghai yinghai yinghai 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

@gujinghui @yinghai @ezyang @facebook-github-bot