Skip to content
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

[proto] Ported SimpleCopyPaste transform #6451

Merged
merged 13 commits into from
Aug 23, 2022
Merged

Conversation

vfdev-5
Copy link
Collaborator

@vfdev-5 vfdev-5 commented Aug 18, 2022

  • Ported SimpleCopyPaste
  • Add tests

Example with MSCoco (batchsize = 2):

image

image

Comment on lines 307 to 318
# fetch all images, bboxes, masks and labels from unstructured input
# with List[image], List[BoundingBox], List[SegmentationMask], List[Label]
images, bboxes, masks, labels = [], [], [], []
for obj in flat_sample:
if isinstance(obj, PIL.Image.Image, features.Image) or is_simple_tensor(obj):
images.append(F.to_image_tensor(obj))
elif isinstance(obj, features.BoundingBox):
bboxes.append(obj)
elif isinstance(obj, features.SegmentationMask):
masks.append(obj)
elif isinstance(obj, (features.Label, features.OneHotLabel)):
labels.append(obj)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As our input is a non-collated batch of datapoint, we need to fetch images, bboxes, masks, labels.
We then transform them and finally put into their places.
I'm not a fan of this code but this helps to do what we want. Maybe we could specify a bit more the input structure and thus code forward in a more elegant way.

Copy link
Collaborator

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

Not a fan of forward behavior here, but I don't think we can really improve it unless we pose some constraints on the input.

output_targets.append(output_data)

# Insert updated images and targets into input flat_sample
c0, c1, c2, c3 = 0, 0, 0, 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't we save these indices the first time we iterate over the flat sample? If yes, maybe we can get away with only doing flat_sample[i] = output[i] and only looping over the output.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, we can do that. I just found that passing indices everywhere would be a bit bulky...

torchvision/prototype/transforms/_augment.py Outdated Show resolved Hide resolved
torchvision/prototype/transforms/_augment.py Outdated Show resolved Hide resolved
@vfdev-5 vfdev-5 marked this pull request as ready for review August 22, 2022 14:35
@vfdev-5 vfdev-5 requested a review from datumbox August 22, 2022 15:02
datumbox
datumbox previously approved these changes Aug 22, 2022
Copy link
Contributor

@datumbox datumbox 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! I left 2 comments below nothing blocking.

torchvision/prototype/transforms/_augment.py Show resolved Hide resolved
torchvision/prototype/transforms/_augment.py Outdated Show resolved Hide resolved
@datumbox
Copy link
Contributor

@vfdev-5 The static typing job is complaining. Seems related.

@datumbox datumbox dismissed their stale review August 23, 2022 08:57

potential bug introduced after review

@vfdev-5
Copy link
Collaborator Author

vfdev-5 commented Aug 23, 2022

Seems like git merge has failed as on main we removed PIL and here I'm still using it.

Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

LGTM again. Github was showing me random stuff on the views BTW. I had to check the whole file to ensure I get the right version. Feel free to merge on green CI.

@vfdev-5 vfdev-5 merged commit c6a715c into pytorch:main Aug 23, 2022
@vfdev-5 vfdev-5 deleted the proto-copy-paste branch August 23, 2022 09:41
@github-actions
Copy link

Hey @vfdev-5!

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

facebook-github-bot pushed a commit that referenced this pull request Aug 25, 2022
Summary:
* WIP

* [proto] Added SimpleCopyPaste transform

* Refactored and cleaned the implementation and added tests

* Fixing code

* Fixed code formatting issue

* Minor updates

* Fixed merge issue

Reviewed By: datumbox

Differential Revision: D39013674

fbshipit-source-id: 212f54c4fd9cbbc72011dc331de61106842d0e99

Co-authored-by: Philip Meier <github.pmeier@posteo.de>
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.

4 participants