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

Dispatch take_along_axis to gather #107711

Closed
wants to merge 4 commits into from

Conversation

Gather does the same thing, but it's much better supported in the
`torch.compile` stack

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Aug 22, 2023

🔗 Helpful Links

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

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

✅ No Failures

As of commit e8d25cd with merge base ba5eeed (image):
💚 Looks good so far! There are no failures yet. 💚

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

Gather does the same thing, but it's much better supported in the
`torch.compile` stack

cc mruberry rgommers voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx chenyang78 aakhundov

[ghstack-poisoned]
Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

tests?

Gather does the same thing, but it's much better supported in the
`torch.compile` stack

cc mruberry rgommers voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx chenyang78 aakhundov

[ghstack-poisoned]
Comment on lines -2416 to -2419
yield SampleInput(
make_arg(),
0,
torch.tensor([0], dtype=torch.int64, device=device))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Repeated sample.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@lezcano one of these is torch.tensor([0]) (i.e. 1d) and the other is torch.tensor(0) (i.e. 0d)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, right, sorry for that. Reverting at #107776

@lezcano
Copy link
Collaborator Author

lezcano commented Aug 22, 2023

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Aug 22, 2023
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

Gather does the same thing, but it's much better supported in the
`torch.compile` stack

cc mruberry rgommers voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx chenyang78 aakhundov

[ghstack-poisoned]
@lezcano
Copy link
Collaborator Author

lezcano commented Aug 22, 2023

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

pytorchmergebot pushed a commit that referenced this pull request Aug 23, 2023
As per title. Tests in the next PR

Pull Request resolved: #107746
Approved by: https://github.com/ezyang
ghstack dependencies: #107687, #107688, #107710, #107711
pytorchmergebot pushed a commit that referenced this pull request Aug 23, 2023
@ev-br
Copy link
Collaborator

ev-br commented Aug 24, 2023

This seems to fail on main locally:

$ pytest test/torch_np/numpy_tests/lib/test_shape_base_.py
Test session starts (platform: linux, Python 3.8.17, pytest 7.4.0, pytest-sugar 0.9.7)
rootdir: /home/ev-br/repos/pytorch
configfile: pytest.ini
plugins: hypothesis-6.82.6, sugar-0.9.7, xdist-3.3.1
collected 72 items                                                                                                                                                                                        

 test/torch_np/numpy_tests/lib/test_shape_base_.py ✓✓✓                                                                                                                                        4% ▌         

―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― TestTakeAlongAxis.test_broadcast ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
Traceback (most recent call last):
  File "/home/ev-br/.conda/envs/pytorch-dev/lib/python3.8/site-packages/_pytest/runner.py", line 341, in from_call
    result: Optional[TResult] = func()
  File "/home/ev-br/.conda/envs/pytorch-dev/lib/python3.8/site-packages/_pytest/runner.py", line 262, in <lambda>
    lambda: ihook(item=item, **kwds), when=when, reraise=reraise
  File "/home/ev-br/.conda/envs/pytorch-dev/lib/python3.8/site-packages/pluggy/_hooks.py", line 433, in __call__
    return self._hookexec(self.name, self._hookimpls, kwargs, firstresult)
  File "/home/ev-br/.conda/envs/pytorch-dev/lib/python3.8/site-packages/pluggy/_manager.py", line 112, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
  File "/home/ev-br/.conda/envs/pytorch-dev/lib/python3.8/site-packages/pluggy/_callers.py", line 155, in _multicall
    return outcome.get_result()
  File "/home/ev-br/.conda/envs/pytorch-dev/lib/python3.8/site-packages/pluggy/_result.py", line 108, in get_result
    raise exc.with_traceback(exc.__traceback__)
  File "/home/ev-br/.conda/envs/pytorch-dev/lib/python3.8/site-packages/pluggy/_callers.py", line 80, in _multicall
    res = hook_impl.function(*args)
  File "/home/ev-br/.conda/envs/pytorch-dev/lib/python3.8/site-packages/_pytest/runner.py", line 177, in pytest_runtest_call
    raise e
  File "/home/ev-br/.conda/envs/pytorch-dev/lib/python3.8/site-packages/_pytest/runner.py", line 169, in pytest_runtest_call
    item.runtest()
  File "/home/ev-br/.conda/envs/pytorch-dev/lib/python3.8/site-packages/_pytest/python.py", line 1788, in runtest
    self.ihook.pytest_pyfunc_call(pyfuncitem=self)
  File "/home/ev-br/.conda/envs/pytorch-dev/lib/python3.8/site-packages/pluggy/_hooks.py", line 433, in __call__
    return self._hookexec(self.name, self._hookimpls, kwargs, firstresult)
  File "/home/ev-br/.conda/envs/pytorch-dev/lib/python3.8/site-packages/pluggy/_manager.py", line 112, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
  File "/home/ev-br/.conda/envs/pytorch-dev/lib/python3.8/site-packages/pluggy/_callers.py", line 116, in _multicall
    raise exception.with_traceback(exception.__traceback__)
  File "/home/ev-br/.conda/envs/pytorch-dev/lib/python3.8/site-packages/pluggy/_callers.py", line 80, in _multicall
    res = hook_impl.function(*args)
  File "/home/ev-br/.conda/envs/pytorch-dev/lib/python3.8/site-packages/_pytest/python.py", line 194, in pytest_pyfunc_call
    result = testfunction(**testargs)
  File "/home/ev-br/repos/pytorch/test/torch_np/numpy_tests/lib/test_shape_base_.py", line 101, in test_broadcast
    actual = take_along_axis(a, ai, axis=1)
  File "/home/ev-br/repos/pytorch/torch/_numpy/_normalizations.py", line 213, in wrapped
    result = func(*args, **kwds)
  File "/home/ev-br/repos/pytorch/torch/_numpy/_funcs_impl.py", line 889, in take_along_axis
    return torch.gather(arr, axis, indices)
RuntimeError: Size does not match at dimension 2 expected index [1, 2, 5] to be smaller than self [3, 4, 1] apart from dimension 1

 test/torch_np/numpy_tests/lib/test_shape_base_.py ⨯✓x✓✓xxxxxxxxx✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓sx✓✓✓✓✓✓✓✓✓✓✓x                                                                    100% ██████████
========================================================================================= short test summary info =========================================================================================
FAILED [0.0032s] test/torch_np/numpy_tests/lib/test_shape_base_.py::TestTakeAlongAxis::test_broadcast - RuntimeError: Size does not match at dimension 2 expected index [1, 2, 5] to be smaller than self [3, 4, 1] apart from dimension 1

Results (0.51s):
      58 passed
       1 failed
         - test/torch_np/numpy_tests/lib/test_shape_base_.py:97 TestTakeAlongAxis.test_broadcast
      12 xfailed
       1 skipped

Reverting the change from take_along_dim to torch.gather fixes the failure.

@lezcano
Copy link
Collaborator Author

lezcano commented Aug 24, 2023

It does! Let me put up a forward fix. This means that not all the tests in torch_np/ are run on CI. @albanD could you give us a hand here? What'd be the best way to add these tests to CI?

lezcano added a commit that referenced this pull request Aug 24, 2023
Addresses #107711 (comment)

I'll have someone submit a decomposition of `take_along_dim` next week.

[ghstack-poisoned]
lezcano added a commit that referenced this pull request Aug 24, 2023
Addresses #107711 (comment)

I'll have someone submit a decomposition of `take_along_dim` next week.

ghstack-source-id: f59e75463653db9086a519c46165afe1ce926136
Pull Request resolved: #107875
@facebook-github-bot facebook-github-bot deleted the gh/Lezcano/225/head branch August 26, 2023 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk Trigger trunk jobs on your pull request Merged module: dynamo module: numpy Related to numpy support, and also numpy compatibility of our operators open source release notes: dynamo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants