Skip to content

Vectorize box encoding in FCOS #6278

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.

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

Merged
merged 8 commits into from
Jul 25, 2022
Merged

Conversation

abhi-glitchhg
Copy link
Contributor

@abhi-glitchhg abhi-glitchhg commented Jul 16, 2022

Just like #5247, i think it is possible to vectorize the following loop at this line.

  # ctrness loss
   bbox_reg_targets = [
       self.box_coder.encode_single(anchors_per_image, boxes_targets_per_image)
       for anchors_per_image, boxes_targets_per_image in zip(anchors, all_gt_boxes_targets)
   ]

Here is my attempt.

Copy link
Contributor

@jdsgomes jdsgomes left a comment

Choose a reason for hiding this comment

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

Hi @abhi-glitchhg, thanks for working on this.
On a first pass looks good. But it is important to test that the implementation is equivalent and also test the performance, would you be able to provide these test scripts? If you need support to run benchmark in GPU for instance I am happy to help

@abhi-glitchhg
Copy link
Contributor Author

abhi-glitchhg commented Jul 19, 2022

@jdsgomes thanks for the review.

I have checked this implementation on the CPU device only.

import time
import torch
from torchvision.models.detection._utils import BoxLinearCoder

device = "cpu"
boxes = [torch.randn(3, 4).to(device=device) for i in range(5)]
rel_codes = [torch.randn( 3, 4).to(device=device) for i in range(5)]

box_coder = BoxLinearCoder(True)

# Warmup

for _ in range(1000):
    original = [
            box_coder.encode_single(bbox_regression_per_image, anchors_per_image)
            for anchors_per_image, bbox_regression_per_image in zip(boxes, rel_codes)
        ]
    original = torch.stack(original)
    new = box_coder.encode_all(rel_codes, boxes)

    torch.testing.assert_close(original, new, rtol=0, atol=1e-6)


start = time.process_time()
for _ in range(10000):
    original = [
            box_coder.encode_single(bbox_regression_per_image, anchors_per_image)
            for anchors_per_image, bbox_regression_per_image in zip(boxes, rel_codes)
        ]
    original = torch.stack(original)
print(f"time taken for encode_single: {time.process_time() -start}")

start = time.process_time()
for _ in range(10000):
    new = box_coder.encode_all(rel_codes, boxes)

print(f"time taken for encode_all: {time.process_time()-start}")

print('done ')

Running the above script, I am getting around 5x improvement.
image

Could you please check the same on GPU?

BTW this script is a slight modification of @datumbox's script in #6203 (review) :)

@jdsgomes
Copy link
Contributor

Could you please check the same on GPU?

thanks for the confirmation. I did a benchmark using GPU and got similar results:

time taken for encode_single: 7.712065194
time taken for encode_all: 1.5909656589999983
done

Copy link
Contributor

@jdsgomes jdsgomes left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for working on this

@jdsgomes jdsgomes merged commit ba0d665 into pytorch:main Jul 25, 2022
@github-actions
Copy link

Hey @jdsgomes!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

@jdsgomes jdsgomes added module: models Perf For performance improvements labels Jul 26, 2022
facebook-github-bot pushed a commit that referenced this pull request Jul 28, 2022
Summary:
* intial structure

* fixed types of few variables

* remove the commented code

* list -> List

* encode method will take input as tensors instead of list of tensor

Reviewed By: datumbox

Differential Revision: D38154574

fbshipit-source-id: ee4936b9968fc1b0cf751c4884c3d5c8064e7d10

Co-authored-by: Joao Gomes <[email protected]>
@abhi-glitchhg abhi-glitchhg deleted the vectorize_encode branch August 8, 2022 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed module: models Perf For performance improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants