Skip to content

[Caffe2] Fix issues in IDEEP fallback ops and enable Detectron on IDEEP device#9164

Closed
gujinghui wants to merge 1 commit intopytorch:masterfrom
gujinghui:detectron
Closed

[Caffe2] Fix issues in IDEEP fallback ops and enable Detectron on IDEEP device#9164
gujinghui wants to merge 1 commit intopytorch:masterfrom
gujinghui:detectron

Conversation

@gujinghui
Copy link
Collaborator

@gujinghui gujinghui commented Jul 4, 2018

Fix issues in IDEEP fallback ops and enable Detectron on IDEEP device

Fix the correctness issue in fallback op implementation when the fallback op is in-place and its upstream op is with IDEEP device. The reorder would happen incorrectly from the same buffer then.
Support Mask-RCNN model from the Detectron, added all the fallbacks needed, e.g. Python op and other related ops. And also support feeding blobs with integer types using TensorCPU.

@gujinghui
Copy link
Collaborator Author

@yinghai @jgong5
Pls help review.

@soumith soumith added the caffe2 label Jul 5, 2018
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.

Hi @gujinghui, what issue does this PR fix and could you outline what the fix is?

Also please clang-format your code.

@jgong5
Copy link
Collaborator

jgong5 commented Jul 5, 2018

@yinghai Below are the fixes and improvements from this PR:

  1. Fix the correctness issue in fallback op implementation when the fallback op is in-place and its upstream op is with IDEEP device. The reorder would happen incorrectly from the same buffer then.
  2. Support Mask-RCNN model from the Detectron, added all the fallbacks needed, e.g. Python op and other related ops. And also support feeding blobs with integer types using TensorCPU.

This comment was marked as off-topic.

This comment was marked as off-topic.

@gujinghui
Copy link
Collaborator Author

gujinghui commented Jul 6, 2018

@yinghai

Added some comments and clang-format the codes.
Pls review.

BTW, planned to clang-format all ideep-related code after PR these patches in hand.
Otherwise, too conflict with present patches.

@gujinghui
Copy link
Collaborator Author

@yinghai pls help review and merge.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@gujinghui
Copy link
Collaborator Author

@yinghai rebased and fixed. pls help review.

@yinghai
Copy link
Contributor

yinghai commented Jul 17, 2018

Still conflict with master?

@gujinghui
Copy link
Collaborator Author

@yinghai
fixed. code base changed.

This comment was marked as off-topic.

This comment was marked as off-topic.

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
Copy link
Contributor

yinghai commented Jul 19, 2018

Please rebase as the failure tests are blocking merge. And do you have any tests for the in-place fallback op?

1. Fix the correctness issue in fallback op implementation when the
fallback op is in-place and its upstream op is with IDEEP device. The
reorder would happen incorrectly from the same buffer then.

2. Support Mask-RCNN model from the Detectron, added all the fallbacks
needed, e.g. Python op and other related ops. And also support feeding
blobs with integer types using TensorCPU.

Signed-off-by: Gu, Jinghui <jinghui.gu@intel.com>
@gujinghui
Copy link
Collaborator Author

rebased & added test case. @yinghai

@jgong5
Copy link
Collaborator

jgong5 commented Jul 27, 2018

@yinghai Any comments on the updates? OK to merge?

@jgong5
Copy link
Collaborator

jgong5 commented Jul 27, 2018

@yinghai Hold on. We need to further polish the fix.

@gujinghui
Copy link
Collaborator Author

Move to other PR

@gujinghui gujinghui closed this Aug 2, 2018
facebook-github-bot pushed a commit that referenced this pull request Aug 29, 2018
Summary:
1. Support ops needed for inference of Faster-RCNN/Mask-RCNN needed in Detectron, mostly direct fallbacks.
2. Use CPU device to hold 0-dim tensors and integer tensors in both fallback op and blob feeder, needed by Detectron models.
3. Ignore 0-dim tensor in MKL-DNN concat operator.
4. Generate dynamic library of Detectron module for CPU device.

This PR obsoletes #9164.
Pull Request resolved: #10157

Differential Revision: D9276837

Pulled By: yinghai

fbshipit-source-id: dc364932ae4a2e7fcefdee70b5fce3c0cee91b6f
PenghuiCheng pushed a commit to PenghuiCheng/pytorch that referenced this pull request Sep 11, 2018
…0157)

Summary:
1. Support ops needed for inference of Faster-RCNN/Mask-RCNN needed in Detectron, mostly direct fallbacks.
2. Use CPU device to hold 0-dim tensors and integer tensors in both fallback op and blob feeder, needed by Detectron models.
3. Ignore 0-dim tensor in MKL-DNN concat operator.
4. Generate dynamic library of Detectron module for CPU device.

This PR obsoletes pytorch#9164.
Pull Request resolved: pytorch#10157

Differential Revision: D9276837

Pulled By: yinghai

fbshipit-source-id: dc364932ae4a2e7fcefdee70b5fce3c0cee91b6f
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.

6 participants