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

Add gallery example for MixUp and CutMix #7772

Merged
merged 7 commits into from
Jul 31, 2023
Merged

Conversation

NicolasHug
Copy link
Member

Closes #7766

@NicolasHug NicolasHug requested a review from pmeier July 28, 2023 11:07
@pytorch-bot
Copy link

pytorch-bot bot commented Jul 28, 2023

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/7772

Note: Links to docs will display an error until the docs builds have been completed.

❌ 32 New Failures

As of commit a367808:

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

"""

# %%
Copy link
Member Author

Choose a reason for hiding this comment

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

Using # %% instead of the long #####... as in the other examples allows for those scripts to be properly interpreted as notebook within vscode / pycharm. It makes writing / debugging those examples a lot easier. I'm tempted to align the other scripts to use this syntax as well?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, sure, but please in a follow-up PR.

docs/Makefile Outdated
@@ -6,7 +6,7 @@ ifneq ($(EXAMPLES_PATTERN),)
endif

# You can set these variables from the command line.
SPHINXOPTS = -W -j auto $(EXAMPLES_PATTERN_OPTS)
SPHINXOPTS = -j auto $(EXAMPLES_PATTERN_OPTS)
Copy link
Member Author

Choose a reason for hiding this comment

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

will put back before merging

Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't forget.

"""

# %%
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, sure, but please in a follow-up PR.

Comment on lines 206 to +207
class Mixup(_BaseMixupCutmix):
"""[BETA] Apply Mixup to the provided batch of images and labels.
"""[BETA] Apply MixUp to the provided batch of images and labels.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we than also rename the classes?

Copy link
Member Author

Choose a reason for hiding this comment

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

thought about it, went for consistency with timm (i.e. Cutmix). No strong opinion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't mind either, but I want both our documentation and class name to be the same. Your choice.

Copy link
Member Author

Choose a reason for hiding this comment

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

🥲 #7731 (comment) I'm going to let you guys decide what to do as I want to avoid re-undo stuff.

(I don't think we absolutely have to align docs and code: CutMix is the technic, Cutmix is the class object - it's OK to have a distinction.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@vfdev-5 Could you clarify if you meant to only switch to MixUp / CutMix for the documentation or for the classes as well? I prefer to have both the same, but don't care whether it is Cutmix or CutMix.

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.

Thanks Nicolas!

Comment on lines 206 to +207
class Mixup(_BaseMixupCutmix):
"""[BETA] Apply Mixup to the provided batch of images and labels.
"""[BETA] Apply MixUp to the provided batch of images and labels.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@vfdev-5 Could you clarify if you meant to only switch to MixUp / CutMix for the documentation or for the classes as well? I prefer to have both the same, but don't care whether it is Cutmix or CutMix.

docs/Makefile Outdated
@@ -6,7 +6,7 @@ ifneq ($(EXAMPLES_PATTERN),)
endif

# You can set these variables from the command line.
SPHINXOPTS = -W -j auto $(EXAMPLES_PATTERN_OPTS)
SPHINXOPTS = -j auto $(EXAMPLES_PATTERN_OPTS)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't forget.

@NicolasHug NicolasHug merged commit 9b4ec8d into pytorch:main Jul 31, 2023
@NicolasHug
Copy link
Member Author

Merged, will address whatever we decide on #7772 later

facebook-github-bot pushed a commit that referenced this pull request Aug 25, 2023
Reviewed By: matteobettini

Differential Revision: D48642291

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

Add example gallery for Mixup and Cutmix
3 participants