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

Tag functions to core IR in native_functions.yaml #105849

Closed
wants to merge 1 commit into from

Conversation

SS-JIA
Copy link
Contributor

@SS-JIA SS-JIA commented Jul 24, 2023

Summary:
Based on operator review meetings, tag appropriate functions as part of the Core IR.

Operator Review Tracking Sheet

Test Plan: Use N3940835 to load the YAML and check updated core op list.

Reviewed By: mergennachin, kimishpatel, SherlockNoMad

Differential Revision: D47673670

@pytorch-bot
Copy link

pytorch-bot bot commented Jul 24, 2023

🔗 Helpful Links

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

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

❌ 2 New Failures

As of commit 866f3dd:

NEW FAILURES - The following jobs have failed:

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

@github-actions
Copy link
Contributor

This PR needs a release notes: label

If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D47673670

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D47673670

@SherlockNoMad
Copy link
Contributor

FYI, you might also need to update test/expect/HasDecompTest.test_aten_core_operators.expect

see #105673 for ref.

@SherlockNoMad SherlockNoMad self-requested a review July 24, 2023 17:24
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D47673670

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D47673670

Copy link
Collaborator

@jgong5 jgong5 left a comment

Choose a reason for hiding this comment

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

It seems not all tagged ops are documented in the spec https://pytorch.org/docs/stable/ir.html. Will the doc be updated accordingly? And does the "core IR" contain ops in both ATen IR and Prims IR?

Comment on lines 1210 to +1211
CompositeExplicitAutograd: logical_xor
tags: pointwise
tags: [core, pointwise]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems logical_xor is missing from the spec? https://pytorch.org/docs/stable/ir.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, so to clarify; that page is generated based on which ops are tagged as "core" from this YAML file. So once this is updated, it should be reflected in the page as well. The list of ops on the page is currently incomplete, and at the moment we are working on expanding the list. Hence this PR.

@@ -1919,6 +1919,7 @@
structured_delegate: cumsum.out
device_check: NoCheck # TensorIterator
variants: function, method
tags: core
Copy link
Collaborator

Choose a reason for hiding this comment

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

cumsum is also not in the spec?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See above comment.

@jgong5
Copy link
Collaborator

jgong5 commented Jul 26, 2023

cc @sujoysaraswati

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D47673670

Summary:
Pull Request resolved: pytorch#105849

Based on operator review meetings, tag appropriate functions as part of the Core IR.

[Operator Review Tracking Sheet](https://docs.google.com/spreadsheets/d/1u9jQ-uGlKu-fe9nLy-jS2AIPtpE8sGTmELOFYgKOhXU/edit#gid=0)

Test Plan: Use N3940835 to load the YAML and check updated core op list.

Reviewed By: mergennachin, kimishpatel, SherlockNoMad

Differential Revision: D47673670

fbshipit-source-id: e3b1afff0318451ce75d403a96844b0aee7d93a4
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D47673670

@facebook-github-bot
Copy link
Contributor

@pytorchbot merge

(Initiating merge automatically since Phabricator Diff has merged)

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

bobby-palmer pushed a commit to bobby-palmer/pytorch that referenced this pull request Jul 29, 2023
Summary:
Based on operator review meetings, tag appropriate functions as part of the Core IR.

[Operator Review Tracking Sheet](https://docs.google.com/spreadsheets/d/1u9jQ-uGlKu-fe9nLy-jS2AIPtpE8sGTmELOFYgKOhXU/edit#gid=0)

Test Plan: Use N3940835 to load the YAML and check updated core op list.

Reviewed By: mergennachin, kimishpatel, SherlockNoMad

Differential Revision: D47673670

Pull Request resolved: pytorch#105849
Approved by: https://github.com/SherlockNoMad
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 fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants