Skip to content

[proto] Simplified code in overridden transform forward methods#6504

Merged
datumbox merged 3 commits intopytorch:mainfrom
vfdev-5:proto-clean-forwards
Aug 26, 2022
Merged

[proto] Simplified code in overridden transform forward methods#6504
datumbox merged 3 commits intopytorch:mainfrom
vfdev-5:proto-clean-forwards

Conversation

@vfdev-5
Copy link
Copy Markdown
Contributor

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

No description provided.

Copy link
Copy Markdown
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, let's wait for the tests to confirm.

Should we apply the changes to _RandomApplyTransform and _auto_augment.py file? cc @pmeier

@pmeier
Copy link
Copy Markdown
Contributor

pmeier commented Aug 26, 2022

Should we apply the changes to _RandomApplyTransform and _auto_augment.py file?

Yes, we should be consistent where we can. AA might be not possible though, because we don't do the regular processing and need to put the augmented image back into the sample.

Edit: Had another look: Since we return early, _RandomApplyTransform is probably also not possible.

@datumbox
Copy link
Copy Markdown
Contributor

@vfdev-5 The failing test is irrelevant; the flaky gaussian blur again. So not a blocker.

@pmeier Sounds good. Could you review this one to see if all the places changed are valid? I think Victor said that he changed everything that is currently safe to do.

@datumbox
Copy link
Copy Markdown
Contributor

The failure is unrelated, merging.

@datumbox datumbox merged commit 13ea901 into pytorch:main Aug 26, 2022
@vfdev-5 vfdev-5 deleted the proto-clean-forwards branch August 26, 2022 12:57
facebook-github-bot pushed a commit that referenced this pull request Aug 30, 2022
…ods (#6504)

Summary: Co-authored-by: Vasilis Vryniotis <datumbox@users.noreply.github.com>

Reviewed By: NicolasHug

Differential Revision: D39131013

fbshipit-source-id: 09cfac00e660c389191caaf917cce8b95d19f151
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