Skip to content

Conversation

pmeier
Copy link
Owner

@pmeier pmeier commented Apr 3, 2023

No description provided.

Comment on lines 280 to 291
Summaries

v2 / v1
Tensor 1.66
PIL 1.54

x / PIL, v1
Tensor, v1 0.92
Tensor, v2 1.53
PIL, v1 1.00
PIL, v2 1.54
Datapoint, v2 1.52
Copy link
Owner Author

Choose a reason for hiding this comment

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

highlighting

transforms.py Outdated
wrapper_factory = WRAPPER_FACTORIES[datasets.CocoDetection]
mock_dataset = SimpleNamespace(ids=list(range(num_samples)))
wrapper = wrapper_factory(mock_dataset)
self.wrapper = functools.partial(wrapper, num_samples // 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why num_samples // 2 and why is it hardcoded to 117_266 above?

Copy link
Owner Author

Choose a reason for hiding this comment

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

This whole thing is mocking the dataset so we can re-use the dataset wrappers as transform. num_samples = 117_266 is the number of samples in the filtered COCO dataset so each sample has one annotation.

In the wrapper, we get passed the idx of the sample

https://github.com/pytorch/vision/blob/39712558247e0d806ceb0df384a6258aa744f950/torchvision/datapoints/_dataset_wrapper.py#L149

and use it like this:

https://github.com/pytorch/vision/blob/39712558247e0d806ceb0df384a6258aa744f950/torchvision/datapoints/_dataset_wrapper.py#L284

Since we don't have access to the idx here, I chose a proxy. Since ids is a list, I used the half of the maximum, to have on average the same performance as doing the correct lookup.

self.stdout.write(message)
self.file.write(message)

def flush(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

where is this needed?

transforms.py Outdated
return image, target


class DetectionReferenceRandomHorizontalFlipV1(transforms_v1.RandomHorizontalFlip):
Copy link
Contributor

Choose a reason for hiding this comment

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

In order to ease per-transform comparison visually, perhaps keep the shorter name RandomHorizontalFlipV1 (same for other transforms).

Ideally we would even drop the V1/V2 suffix

@pmeier
Copy link
Owner Author

pmeier commented Apr 4, 2023

We identified the slowdown from detection v2 stems from two things:

  1. pytree is comparatively really slow for "large" inputs. Since by default, CocoDetection includes a nested list of coordinates. Although they are not transformed, they also need to be flattened and unflattened.
  2. Keeping the masks in the sample means they will be transformed although we don't need them.

Results here are run on torchvision nightly April 4. When pytorch/vision#7488 is merged and pytorch/vision#7489 addressed, we need to re-run.

@pmeier pmeier merged commit 0ae9027 into main Apr 4, 2023
@pmeier pmeier deleted the detection branch April 4, 2023 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants