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

ENH Adds no_batch_dim support for pad 2d and 3d #62183

Closed
wants to merge 1 commit into from

Conversation

thomasjpfan
Copy link
Contributor

Towards #60585

@thomasjpfan thomasjpfan added module: nn Related to torch.nn triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels Jul 26, 2021
@thomasjpfan thomasjpfan requested a review from albanD as a code owner July 26, 2021 14:02
@thomasjpfan thomasjpfan added this to In Progress in torch.nn via automation Jul 26, 2021
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jul 26, 2021

🔗 Helpful links

💊 CI failures summary and remediations

As of commit b61ed60 (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

Comment on lines +583 to +584
input_inner = input.unsqueeze(0);
output_inner = output.unsqueeze(0);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a bug fix where the no_batch_dim test serves as a non-regression test.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to clarify, the bug kept non-batch mode from functioning properly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to clarify, the bug kept non-batch mode from functioning properly?

Yup. In this case, "non-batch mode" == "no batch dims". Now that we have better language for batch dims, I would want to renamed the batch_mode to has_batch_dim.

Comment on lines +676 to +677
input_ = input.unsqueeze(0);
output_ = output.unsqueeze(0);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here

@albanD albanD removed their request for review July 26, 2021 14:49
Copy link
Contributor

@jbschlosser jbschlosser left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines +583 to +584
input_inner = input.unsqueeze(0);
output_inner = output.unsqueeze(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to clarify, the bug kept non-batch mode from functioning properly?

@facebook-github-bot
Copy link
Contributor

@jbschlosser has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

torch.nn automation moved this from In Progress to Done Jul 28, 2021
@facebook-github-bot
Copy link
Contributor

@jbschlosser merged this pull request in 7c588d5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed Merged module: nn Related to torch.nn open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
torch.nn
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants