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.

Sign up for GitHub

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

Jump to bottom

Added rot90 batch rule #138

Merged
merged 6 commits into from
Sep 26, 2021
Merged

Added rot90 batch rule #138

merged 6 commits into from
Sep 26, 2021

Conversation

vfdev-5
Copy link
Contributor

@vfdev-5 vfdev-5 commented Sep 23, 2021 •
edited
Loading

Description:

  • Added rot90 batch rule
  • Enabled associated tests

Related to #112

Note: tests seem not be reliable to output bdim.
Tests are passing even if batching rule outputs wrong bdim

@vfdev-5
Added rot90 batch rule
6cd754a 
Description:
- Added rot90 batch rule
- Enabled associated tests

Note: tests seem not be reliable to output bdim.
Tests are passing even if batching rule outputs wrong bdim
@zou3519
Copy link
Contributor

zou3519 commented Sep 23, 2021

Note: tests seem not be reliable to output bdim.
Tests are passing even if batching rule outputs wrong bdim

Yeah our vmap tests don't test for non-zero bdim. We should fix that at some point...

@Chillee
Copy link
Contributor

Chillee commented Sep 23, 2021

Does this work with REDUCTION_BOXED?

Specifically, REDUCTION_BOXED_ARGS(rot90, 2) (which specifies that the dim argument is at position 2).

@vfdev-5
Copy link
Contributor Author

vfdev-5 commented Sep 24, 2021 •
edited
Loading

@Chillee I like this magical macro ! Yes, seems like tests are passing with REDUCTION_BOXED_ARGS and without writing any batching rule :)

Let me update the PR.

The only point here is that REDUCTION_BOXED_ARGS is exposed in BatchRulesReduceOps.cpp but rot90 was expected to go to BatchRulesViews.cpp.

@zou3519 zou3519 mentioned this pull request Sep 24, 2021
Closed
@Chillee
Copy link
Contributor

Chillee commented Sep 25, 2021 •
edited
Loading

There's an xfail now passing - I suspect you need to also remove the test from the has_batch_rule OpInfo test.

@Chillee
Copy link
Contributor

Chillee commented Sep 26, 2021

LGTM. Thanks!

@Chillee Chillee merged commit dd23337 into pytorch:main Sep 26, 2021
@vfdev-5 vfdev-5 deleted the add-rot90-br branch September 26, 2021 07:00
@vfdev-5 vfdev-5 mentioned this pull request Oct 4, 2021
Merged
zou3519 pushed a commit that referenced this pull request Oct 22, 2021
@vfdev-5 @zou3519
Added rot90 batch rule (#138)
a99a86a 
* Added rot90 batch rule
Description:
- Added rot90 batch rule
- Enabled associated tests

Note: tests seem not be reliable to output bdim.
Tests are passing even if batching rule outputs wrong bdim

* Replaced manual batching rule with REDUCTION_BOXED_ARGS

* Removed commented xfail('rot90')
zou3519 pushed a commit to zou3519/pytorch that referenced this pull request Jul 20, 2022
@vfdev-5 @zou3519
[functorch] Added rot90 batch rule (pytorch/functorch#138)
97766f4 
* Added rot90 batch rule
Description:
- Added rot90 batch rule
- Enabled associated tests

Note: tests seem not be reliable to output bdim.
Tests are passing even if batching rule outputs wrong bdim

* Replaced manual batching rule with REDUCTION_BOXED_ARGS

* Removed commented xfail('rot90')
bigfootjon pushed a commit to pytorch/pytorch that referenced this pull request Jul 21, 2022
@vfdev-5 @bigfootjon
[functorch] Added rot90 batch rule (pytorch/functorch#138)
53af441 
* Added rot90 batch rule
Description:
- Added rot90 batch rule
- Enabled associated tests

Note: tests seem not be reliable to output bdim.
Tests are passing even if batching rule outputs wrong bdim

* Replaced manual batching rule with REDUCTION_BOXED_ARGS

* Removed commented xfail('rot90')
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cla signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vfdev-5 @zou3519 @Chillee @facebook-github-bot