Skip to content

Removes unnecessary namespace of functions used only in einsum #84955

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Removes unnecessary namespace of functions used only in einsum #84955

wants to merge 1 commit into from

Conversation

@janeyx99
Copy link
Contributor

This is cosmetic change that removes a few function declarations and derives values instead of hardcoding. This is step 1 in relanding a cleaner version of einsum with opt_einsum. See #60191

@pytorch-bot
Copy link

pytorch-bot bot commented Sep 13, 2022 •
edited
Loading

🔗 Helpful Links

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

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

✅ No Failures

As of commit 1f85066:
💚 Looks good so far! There are no failures yet. 💚

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

soulitzer
soulitzer approved these changes Sep 13, 2022
View reviewed changes
Copy link
Contributor

@soulitzer soulitzer left a comment

Choose a reason for hiding this comment

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

Cool!

@janeyx99
Copy link
Contributor Author

@pytorchbot merge

@janeyx99 janeyx99 marked this pull request as ready for review September 14, 2022 00:29
@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a merge job. Check the current status here and land check progress here.
The merge job was triggered with the land checks (-l) flag. If you did not specify this flag yourself, you are likely enrolled in the land checks rollout. This means that your change will be merged once all checks on your PR and the land checks have passed (ETA 4 Hours). If you need to coordinate lands between different changes and cannot risk a land race, please add the ciflow/trunk label to your PR and wait for signal to complete, and then land your changes in proper order. Having trunk, pull, and Lint pre-run on a PR will bypass land checks and the ETA should be immediate. If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

@github-actions
Copy link
Contributor

Hey @janeyx99.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

mehtanirav pushed a commit that referenced this pull request Oct 4, 2022
@janeyx99 @mehtanirav
Removes unnecessary namespace of functions used only in einsum (#84955)
0b300a8 
This is cosmetic change that removes a few function declarations and derives values instead of hardcoding. This is step 1 in relanding a cleaner version of einsum with opt_einsum. See #60191
Pull Request resolved: #84955
Approved by: https://github.com/soulitzer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@soulitzer soulitzer soulitzer approved these changes

Assignees

No one assigned

Labels

cla signed Merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@janeyx99 @pytorchmergebot @soulitzer @facebook-github-bot