KEMBAR78
Deprecate torch.cross default behaviour by lezcano · Pull Request #108760 · pytorch/pytorch · GitHub
Skip to content

Conversation

@lezcano
Copy link
Collaborator

@lezcano lezcano commented Sep 7, 2023

Stack from ghstack (oldest at bottom):

Long overdue this one. We may be able to change it in a few years :hopeful:.

BC-breaking note

This PR deprecates torch.cross's default dim in favor of
torch.linalg.cross.
A upgrade guide is added to the documentation for torch.cross.

Note this PR DOES NOT remove torch.cross.

Fixes #108664

cc @jianyuh @nikitaved @pearu @mruberry @walterddr @IvanYashchuk @xwang233 @lezcano

Long overdue this one. We may be able to change it in a few years :hopeful:.

**BC-breaking note**

This PR deprecates `torch.cross`'s default dim in favor of
`torch.linalg.cross`.
A upgrade guide is added to the documentation for `torch.cross`.

Note this PR DOES NOT remove `torch.cross`.

Fixes #108664

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Sep 7, 2023

🔗 Helpful Links

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

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

✅ No Failures

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

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

lezcano added a commit that referenced this pull request Sep 7, 2023
Long overdue this one. We may be able to change it in a few years :hopeful:.

**BC-breaking note**

This PR deprecates `torch.cross`'s default dim in favor of
`torch.linalg.cross`.
A upgrade guide is added to the documentation for `torch.cross`.

Note this PR DOES NOT remove `torch.cross`.

Fixes #108664

ghstack-source-id: fb21a77
Pull Request resolved: #108760
@lezcano lezcano requested a review from albanD September 7, 2023 08:25
@lezcano lezcano added module: linear algebra Issues related to specialized linear algebra operations in PyTorch; includes matrix multiply matmul release notes: linalg_frontend release notes category topic: deprecation topic category labels Sep 7, 2023
@lezcano
Copy link
Collaborator Author

lezcano commented Sep 7, 2023

That Softmax deprecation may be more painful, and it'll need to be done by someone at meta. Feel free to poke them in that other issue.

@vadimkantorov
Copy link
Contributor

vadimkantorov commented Sep 7, 2023

Yeah, for softmax default dim was deprecated for a few years no, but still not removed :) which makes it have a functional wrapper and nasty stacklevel arg

Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

nit on the doc but sounds good otherwise

if (!dimension) {
TORCH_WARN_ONCE(
"Using torch.cross without specifying the dim arg is deprecated.\n",
"Please either pass the dim explicitly or simply use linalg.cross.\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit:

Suggested change
"Please either pass the dim explicitly or simply use linalg.cross.\n",
"Please either pass the dim explicitly or simply use torch.linalg.cross.\n",

Long overdue this one. We may be able to change it in a few years :hopeful:.

**BC-breaking note**

This PR deprecates `torch.cross`'s default dim in favor of
`torch.linalg.cross`.
A upgrade guide is added to the documentation for `torch.cross`.

Note this PR DOES NOT remove `torch.cross`.

Fixes #108664

cc jianyuh nikitaved pearu mruberry walterddr IvanYashchuk xwang233 Lezcano

[ghstack-poisoned]
lezcano added a commit that referenced this pull request Sep 14, 2023
Long overdue this one. We may be able to change it in a few years :hopeful:.

**BC-breaking note**

This PR deprecates `torch.cross`'s default dim in favor of
`torch.linalg.cross`.
A upgrade guide is added to the documentation for `torch.cross`.

Note this PR DOES NOT remove `torch.cross`.

Fixes #108664

ghstack-source-id: 3e7bae9
Pull Request resolved: #108760
@lezcano
Copy link
Collaborator Author

lezcano commented Sep 14, 2023

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 14, 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

@facebook-github-bot facebook-github-bot deleted the gh/Lezcano/236/head branch September 18, 2023 14:23
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: linear algebra Issues related to specialized linear algebra operations in PyTorch; includes matrix multiply matmul open source release notes: linalg_frontend release notes category topic: deprecation topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants