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

Actually actionable list of batching rules to write #240

Open
15 of 16 tasks
zou3519 opened this issue Nov 4, 2021 · 16 comments
Open
15 of 16 tasks

Actually actionable list of batching rules to write #240

zou3519 opened this issue Nov 4, 2021 · 16 comments
Labels
actionable It is clear what should be done for this issue

Comments

@zou3519
Copy link
Contributor

zou3519 commented Nov 4, 2021

For each of the items here, we should make sure all compositions (vmap, vmap x vjp) have a batching rule. All of these items should be actionable (in that it is possible to write a batching rule and we are not blocked on functionalization, which is coming soon).

Note: you may need to write an OpInfo for the operator if it doesn't exist already or wait for one to be added. A lot of folks are adding OpInfos right now, so if the OpInfo doesn't exist please ask first to see if someone is working on it.

Note: if any of the operations decompose into in-place operations, then we need functionalization to handle them. I think I've already filtered out all of those, but please check me on that.

Parcel 1: top nn.functional.* and top torch.* foo

  • torch.nn.functional.interpolate (this involves adding batching rules for adaptive_avg_pool1d, but we might as well do adaptive_avg_pool{1, 2, 3}d as well as their backward variants while we're at it)
  • nn.functional.unfold. Involves writing a batching rule for im2col im2col_backward (Added im2col batch rule and enabled vmap for nn.functional.unfold op #262)
  • nn.functional.grid_sample. Involves writing a batching rule for the backward operator. @vfdev-5
  • torch.pow. The backward needs batching rule for logical_and ; use this as an opportunity to write the batching rules for logical_{and, or, xor} if those don't exist yet. We may need to also add a change to PyTorch core to make the logical_* functions primitives w.r.t. autograd

Parcel 2: new_blah

  • new_empty, new_full, new_ones, new_zeros, empty_like, zeros_like, ones_like, full_like
  • adaptive_max_pool{1, 2, 3}d as well as the backward variants
  • diagonal_scatter, select_scatter, slice_scatter @kshitij12345
  • linalg.householder_product (forward pass only)
  • pixel_shuffle, pixel_unshuffle (these might just be OP_DECOMPOSE)
  • isinf, isfinite, isnan
  • _cdist_forward, _cdist_backward (try to write a batching rule if possible. If not possible, we may need to write a decomposition) @vfdev-5

Parcel 3: linalg things

Parcel 4:

@zou3519 zou3519 added the actionable It is clear what should be done for this issue label Nov 4, 2021
@vfdev-5
Copy link
Contributor

vfdev-5 commented Nov 8, 2021

@zou3519 nn.functional.pad with circular option requires to do a copy: out[..., out_d0:out_d1] = input[..., in_d0:in_d1] and thus there is the following error is raised:

>           out[..., out_d0:out_d1] = input[..., in_d0:in_d1]                                                                                                                               
E           RuntimeError: vmap: aten::copy_(self, *extra_args) is not possible because there exists a Tensor `other` in extra_args that has more elements than `self`. This happened due to `other` being vmapped over but `self` not being vmapped over at level 2. Please try to use out-of-place operators instead of aten::copy_. If said operator is being called inside the PyTorch framework, please file a bug report instead. 

Can we do something with that ?

@zou3519
Copy link
Contributor Author

zou3519 commented Nov 8, 2021

Aha. Nope, we can't do anything about that until functionalization is in, good catch.

vfdev-5 added a commit to vfdev-5/functorch that referenced this issue Nov 9, 2021
Description:
- Added backward batch rule for pad replicate/reflect modes
- Updated tests

Related to pytorch#240
zou3519 pushed a commit that referenced this issue Nov 10, 2021
Description:
- Added backward batch rule for pad replicate/reflect modes
- Updated tests

Related to #240
@zou3519 zou3519 added actionable It is clear what should be done for this issue and removed actionable It is clear what should be done for this issue labels Nov 12, 2021
@zou3519 zou3519 pinned this issue Nov 13, 2021
@Padarn
Copy link
Contributor

Padarn commented Nov 14, 2021

Hi @zou3519, the "forward pass only" ops above means that the vjp and related operators require the functionalization too?

vfdev-5 added a commit to vfdev-5/functorch that referenced this issue Nov 14, 2021
Description:
- Added im2col batch rule and enabled vmap for nn.functional.unfold op
- Updated tests

Using EXISTING_BDIM macro to put bdim into 0 as im2col expects dim=0 to be batch dim

Related to pytorch#240
Chillee pushed a commit that referenced this issue Nov 15, 2021
…262)

Description:
- Added im2col batch rule and enabled vmap for nn.functional.unfold op
- Updated tests

Using EXISTING_BDIM macro to put bdim into 0 as im2col expects dim=0 to be batch dim

Related to #240
vfdev-5 added a commit to vfdev-5/functorch that referenced this issue Nov 15, 2021
Description:
- Added adaptive_max_poolNd fw/bw batch rules
- Updated tests

Related to pytorch#240

Notes:
I created two additional macros to handle adaptive_max_pool2d and adaptive_max_pool3d_backward.
Not sure if we could make a generic rule to handle max_pool2d_with_indices_backward_batch_rule and adaptive_max_pool3d_backward,
as max_pool2d_with_indices_backward_batch_rule requires some args in the middle between gradOutput, input and indices.
@zou3519
Copy link
Contributor Author

zou3519 commented Nov 15, 2021

Hi @zou3519, the "forward pass only" ops above means that the vjp and related operators require the functionalization too?

Yes, "forward pass only" means we should only try to get the vmap tests passing and none of the vjp/grad/compositions of {vjp, grad} tests.

vfdev-5 added a commit to vfdev-5/functorch that referenced this issue Nov 23, 2021
Description:
- Added adaptive_max_poolNd fw/bw batch rules
- Updated tests

Related to pytorch#240

Notes:
I created two additional macros to handle adaptive_max_pool2d and adaptive_max_pool3d_backward.
Not sure if we could make a generic rule to handle max_pool2d_with_indices_backward_batch_rule and adaptive_max_pool3d_backward,
as max_pool2d_with_indices_backward_batch_rule requires some args in the middle between gradOutput, input and indices.
Chillee pushed a commit that referenced this issue Nov 23, 2021
* Added adaptive_max_poolNd fw/bw batch rules
Description:
- Added adaptive_max_poolNd fw/bw batch rules
- Updated tests

Related to #240

Notes:
I created two additional macros to handle adaptive_max_pool2d and adaptive_max_pool3d_backward.
Not sure if we could make a generic rule to handle max_pool2d_with_indices_backward_batch_rule and adaptive_max_pool3d_backward,
as max_pool2d_with_indices_backward_batch_rule requires some args in the middle between gradOutput, input and indices.

* Replaced EXISTING_BDIM_MULTIOUT by EXISTING_BDIM_ALL_BOXED

* Removed specific implementations with indices.contiguous() for
- max_pool2d_with_indices_backward
- adaptive_max_pool2d_backward
- adaptive_max_pool3d_backward
and added ALL_TENSORS_HAVE_OPTIONAL_BDIM_BOXED_CONTIG1 to handle that
@vfdev-5
Copy link
Contributor

vfdev-5 commented Nov 30, 2021

@kshitij12345 on which tasks from Parcel 2 you are working on and plan to work on ?

I can start working on _cdist_forward, _cdist_backward

@kshitij12345
Copy link
Collaborator

@vfdev-5 I think I'll be picking diagonal_scatter next. Go ahead with _cdist_forward, _cdist_backward

@vfdev-5
Copy link
Contributor

vfdev-5 commented Dec 8, 2021

I'll take torch.addr and linalg.eig and cholesky_solve and _lu_with_info next

zou3519 pushed a commit that referenced this issue Dec 13, 2021
Description:
- Added batch rule for LU op
- Updated tests

Related to #240
@vfdev-5
Copy link
Contributor

vfdev-5 commented Dec 23, 2021

To close this issue, it remains to finalize parcel 2:

and in parcel 4:

@zou3519
Copy link
Contributor Author

zou3519 commented Dec 23, 2021

There's always more batching rules to write, I'll put up a new issue for them later :)

@lezcano
Copy link
Contributor

lezcano commented Jan 7, 2022

Note that _lu_with_info is not a thing any more. Now we have linalg_lu_factor and linalg_lu_factor_ex cf. pytorch/pytorch#66933

@vfdev-5
Copy link
Contributor

vfdev-5 commented Jan 10, 2022

linalg_lu_factor

@lezcano thanks for the update ! I see that _lu_with_info is marked to be deprecated, so torch.lu will be deprecated as well ?

@lezcano
Copy link
Contributor

lezcano commented Jan 10, 2022

It will indeed. And that's a good reminder for me to put up a PR doing so :D

@zou3519 zou3519 unpinned this issue Feb 17, 2022
@vfdev-5
Copy link
Contributor

vfdev-5 commented Mar 18, 2022

@zou3519 can we update description list with with was done. I think we can remove Parcel 4 from here and create new issue for that if needed. What remains here is to sync and merge householder product PR (#322), cc @kshitij12345 .

@lezcano
Copy link
Contributor

lezcano commented Mar 18, 2022

Fwiw, following up on the point above on deprecating torch.lu:
pytorch/pytorch#73804
pytorch/pytorch#73806

@zou3519
Copy link
Contributor Author

zou3519 commented Mar 22, 2022

@zou3519 can we update description list with with was done. I think we can remove Parcel 4 from here and create new issue for that if needed. What remains here is to sync and merge householder product PR (#322)

Yes I'll create another issue soon

zou3519 pushed a commit to zou3519/pytorch that referenced this issue Jul 20, 2022
…pytorch/functorch#251)

Description:
- Added backward batch rule for pad replicate/reflect modes
- Updated tests

Related to pytorch/functorch#240
zou3519 pushed a commit to zou3519/pytorch that referenced this issue Jul 20, 2022
…l.unfold op (pytorch/functorch#262)

Description:
- Added im2col batch rule and enabled vmap for nn.functional.unfold op
- Updated tests

Using EXISTING_BDIM macro to put bdim into 0 as im2col expects dim=0 to be batch dim

Related to pytorch/functorch#240
zou3519 pushed a commit to zou3519/pytorch that referenced this issue Jul 20, 2022
* Added adaptive_max_poolNd fw/bw batch rules
Description:
- Added adaptive_max_poolNd fw/bw batch rules
- Updated tests

Related to pytorch/functorch#240

Notes:
I created two additional macros to handle adaptive_max_pool2d and adaptive_max_pool3d_backward.
Not sure if we could make a generic rule to handle max_pool2d_with_indices_backward_batch_rule and adaptive_max_pool3d_backward,
as max_pool2d_with_indices_backward_batch_rule requires some args in the middle between gradOutput, input and indices.

* Replaced EXISTING_BDIM_MULTIOUT by EXISTING_BDIM_ALL_BOXED

* Removed specific implementations with indices.contiguous() for
- max_pool2d_with_indices_backward
- adaptive_max_pool2d_backward
- adaptive_max_pool3d_backward
and added ALL_TENSORS_HAVE_OPTIONAL_BDIM_BOXED_CONTIG1 to handle that
zou3519 pushed a commit to zou3519/pytorch that referenced this issue Jul 20, 2022
Description:
- Added cholesky_solve op batch rule
- Updated tests

Related to pytorch/functorch#240
zou3519 pushed a commit to zou3519/pytorch that referenced this issue Jul 20, 2022
Description:
- Added addr op decomposition
- Updated tests

Related to pytorch/functorch#240
zou3519 pushed a commit to zou3519/pytorch that referenced this issue Jul 20, 2022
Description:
- Added batch rule for LU op
- Updated tests

Related to pytorch/functorch#240
bigfootjon pushed a commit to pytorch/pytorch that referenced this issue Jul 21, 2022
…pytorch/functorch#251)

Description:
- Added backward batch rule for pad replicate/reflect modes
- Updated tests

Related to pytorch/functorch#240
bigfootjon pushed a commit to pytorch/pytorch that referenced this issue Jul 21, 2022
…l.unfold op (pytorch/functorch#262)

Description:
- Added im2col batch rule and enabled vmap for nn.functional.unfold op
- Updated tests

Using EXISTING_BDIM macro to put bdim into 0 as im2col expects dim=0 to be batch dim

Related to pytorch/functorch#240
bigfootjon pushed a commit to pytorch/pytorch that referenced this issue Jul 21, 2022
* Added adaptive_max_poolNd fw/bw batch rules
Description:
- Added adaptive_max_poolNd fw/bw batch rules
- Updated tests

Related to pytorch/functorch#240

Notes:
I created two additional macros to handle adaptive_max_pool2d and adaptive_max_pool3d_backward.
Not sure if we could make a generic rule to handle max_pool2d_with_indices_backward_batch_rule and adaptive_max_pool3d_backward,
as max_pool2d_with_indices_backward_batch_rule requires some args in the middle between gradOutput, input and indices.

* Replaced EXISTING_BDIM_MULTIOUT by EXISTING_BDIM_ALL_BOXED

* Removed specific implementations with indices.contiguous() for
- max_pool2d_with_indices_backward
- adaptive_max_pool2d_backward
- adaptive_max_pool3d_backward
and added ALL_TENSORS_HAVE_OPTIONAL_BDIM_BOXED_CONTIG1 to handle that
bigfootjon pushed a commit to pytorch/pytorch that referenced this issue Jul 21, 2022
Description:
- Added cholesky_solve op batch rule
- Updated tests

Related to pytorch/functorch#240
bigfootjon pushed a commit to pytorch/pytorch that referenced this issue Jul 21, 2022
Description:
- Added addr op decomposition
- Updated tests

Related to pytorch/functorch#240
bigfootjon pushed a commit to pytorch/pytorch that referenced this issue Jul 21, 2022
Description:
- Added batch rule for LU op
- Updated tests

Related to pytorch/functorch#240
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
actionable It is clear what should be done for this issue
Projects
None yet
Development

No branches or pull requests

5 participants