Skip to content

Conversation

Fixes #134278

Test Plan:
- tested locally

[ghstack-poisoned]
Copy link

pytorch-bot bot commented Aug 28, 2024

🔗 Helpful Links

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

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

✅ You can merge normally! (1 Unrelated Failure)

As of commit 761163f with merge base 55236d0 (image):

FLAKY - The following job failed but was likely due to flakiness present on trunk:

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

@zou3519 zou3519 requested review from albanD and yushangdi August 28, 2024 16:12
@zou3519 zou3519 added ciflow/trunk Trigger trunk jobs on your pull request release notes: composability release notes category labels Aug 28, 2024
f"the offending output tensors (e.g. output.clone()) "
f"or refactor your code. "
f"Offending op: {self._name} (with implementation in {module})"
f"{self._name} (with implementation in {module}): "
Copy link
Collaborator

Choose a reason for hiding this comment

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

Aren't inplace ops expected to return the input that they modify inplace as-is though to be able to support autograd?

Copy link
Contributor Author

@zou3519 zou3519 Aug 28, 2024

Choose a reason for hiding this comment

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

We don't support that in the compile path or the torch.library.custom_op API. We can extend support for it in the future but users have been fine without it.

f"or refactor your code. "
f"Offending op: {self._name} (with implementation in {module})"
f"{self._name} (with implementation in {module}): "
f"The output of this custom operator (1) must not "
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
f"The output of this custom operator (1) must not "
f"The output of a custom operator (1) must not "

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using "this" was intentional, people had trouble interpreting it

Fixes #134278

Test Plan:
- tested locally

[ghstack-poisoned]
zou3519 added a commit that referenced this pull request Aug 28, 2024
Fixes #134278

Test Plan:
- tested locally

ghstack-source-id: 06cf8e3
Pull Request resolved: #134688
@zou3519
Copy link
Contributor Author

zou3519 commented Aug 28, 2024

@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

Chao1Han pushed a commit to Chao1Han/pytorch that referenced this pull request Sep 20, 2024
@github-actions github-actions bot deleted the gh/zou3519/1055/head branch October 2, 2024 02:10
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 release notes: composability release notes category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants