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.

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

Allow getting all backend names #8520

Merged
merged 1 commit into from
Feb 21, 2025
Merged

Allow getting all backend names #8520

merged 1 commit into from
Feb 21, 2025

Conversation

cccclai
Copy link
Contributor

@cccclai cccclai commented Feb 17, 2025

Summary: Allow getting all backends name in both python and c++

Differential Revision: D69691354

@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Feb 17, 2025 •
edited
Loading

🔗 Helpful Links

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

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

❌ 1 New Failure

As of commit 26f8fa8 with merge base 80d5e5a (image):

NEW FAILURE - The following job has failed:

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

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 17, 2025
@facebook-github-bot
Copy link
Contributor

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

cccclai added a commit to cccclai/executorch-1 that referenced this pull request Feb 18, 2025
@cccclai @facebook-github-bot
Allow getting all backend names (pytorch#8520)
d1a776b 
Summary:

Allow getting all backends name in both python and c++

Reviewed By: tarun292

Differential Revision: D69691354
@facebook-github-bot
Copy link
Contributor

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

@cccclai cccclai added the release notes: api Changes to public facing apis (any interfaces, pybinded runtime methods, etc.) label Feb 18, 2025
cccclai added a commit to cccclai/executorch-1 that referenced this pull request Feb 18, 2025
@cccclai @facebook-github-bot
Allow getting all backend names (pytorch#8520)
e1a9ab9 
Summary:

Allow getting all backends name in both python and c++

Reviewed By: tarun292

Differential Revision: D69691354
@facebook-github-bot
Copy link
Contributor

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

@larryliu0820
Copy link
Contributor

larryliu0820 commented Feb 18, 2025 •
edited
Loading

We are moving to runtime/__init__.py for our runtime python APIs. Can you add this new API there as well?

larryliu0820
larryliu0820 requested changes Feb 18, 2025
View reviewed changes
Copy link
Contributor

@larryliu0820 larryliu0820 left a comment

Choose a reason for hiding this comment

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

Put it back to your queue

cccclai added a commit to cccclai/executorch-1 that referenced this pull request Feb 18, 2025
@cccclai @facebook-github-bot
Allow getting all backend names (pytorch#8520)
44978f1 
Summary:

Allow getting all backends name in both python and c++

Reviewed By: tarun292

Differential Revision: D69691354
@facebook-github-bot
Copy link
Contributor

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

dbort
dbort requested changes Feb 18, 2025
View reviewed changes
cccclai added a commit to cccclai/executorch-1 that referenced this pull request Feb 18, 2025
@cccclai @facebook-github-bot
Allow getting all backend names (pytorch#8520)
ff92a48 
Summary:

Allow getting all backends name in both python and c++

Reviewed By: tarun292

Differential Revision: D69691354
@facebook-github-bot
Copy link
Contributor

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

@cccclai
Copy link
Contributor Author

cccclai commented Feb 18, 2025

@larryliu0820 @dbort updated and addressed the comments. Please take another look.

cccclai added a commit to cccclai/executorch-1 that referenced this pull request Feb 18, 2025
@cccclai @facebook-github-bot
Allow getting all backend names (pytorch#8520)
01445cc 
Summary:

Allow getting all backends name in both python and c++

Reviewed By: tarun292

Differential Revision: D69691354
@facebook-github-bot
Copy link
Contributor

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

larryliu0820
larryliu0820 approved these changes Feb 18, 2025
View reviewed changes
Copy link
Contributor

@larryliu0820 larryliu0820 left a comment

Choose a reason for hiding this comment

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

Thanks for addressing comments

@cccclai
Copy link
Contributor Author

cccclai commented Feb 18, 2025

This could be done as a follow-up as I wouldn't want to block this diff, but it would be nice to be able to get only available backends - i.e. backends where is_available() returns true. This is useful for determining if the actual hardware is present. For example, the Qualcomm backend might be linked but not running on a device that actually has a supported NPU. The older API that returned the Backend directly would have allowed for calling is_available(), but the current approach does not. That's fine, as we can add another API to do this - just a consideration.

oh I actually already have this diff as part of the follow up. Just haven't submitted yet.

cccclai added a commit to cccclai/executorch-1 that referenced this pull request Feb 18, 2025
@cccclai @facebook-github-bot
Allow getting all backend names (pytorch#8520)
e2fefaf 
Summary:

Allow getting all backends name in both python and c++

Reviewed By: larryliu0820

Differential Revision: D69691354
@facebook-github-bot
Copy link
Contributor

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

cccclai added a commit to cccclai/executorch-1 that referenced this pull request Feb 18, 2025
@cccclai @facebook-github-bot
Allow getting all backend names (pytorch#8520)
75c22a1 
Summary:

Allow getting all backends name in both python and c++

Reviewed By: omerjerk

Differential Revision: D69691354
@facebook-github-bot
Copy link
Contributor

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

mergennachin
mergennachin requested changes Feb 19, 2025
View reviewed changes
cccclai added a commit to cccclai/executorch-1 that referenced this pull request Feb 19, 2025
@cccclai @facebook-github-bot
Allow getting all backend names (pytorch#8520)
f5d658e 
Summary:

Allow getting all backends name in both python and c++

Reviewed By: omerjerk

Differential Revision: D69691354
@facebook-github-bot
Copy link
Contributor

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

cccclai added a commit to cccclai/executorch-1 that referenced this pull request Feb 20, 2025
@cccclai @facebook-github-bot
Allow getting all backend names (pytorch#8520)
3117249 
Summary:

Allow getting all backends name in both python and c++

Reviewed By: omerjerk

Differential Revision: D69691354
@facebook-github-bot
Copy link
Contributor

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

cccclai added a commit to cccclai/executorch-1 that referenced this pull request Feb 21, 2025
@cccclai @facebook-github-bot
Allow getting all backend names (pytorch#8520)
c6eb626 
Summary:

Allow getting all backends name in both python and c++

Reviewed By: omerjerk

Differential Revision: D69691354
@facebook-github-bot
Copy link
Contributor

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

dbort
dbort approved these changes Feb 21, 2025
View reviewed changes
Copy link
Contributor

@dbort dbort left a comment

Choose a reason for hiding this comment

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

Thanks for changing the API to be index-based. Looks good apart from my final request to update the .pyi file.

cccclai added a commit to cccclai/executorch-1 that referenced this pull request Feb 21, 2025
@cccclai @facebook-github-bot
Allow getting all backend names (pytorch#8520)
a41ad12 
Summary:

Allow getting all backends name in both python and c++

Reviewed By: omerjerk

Differential Revision: D69691354
@facebook-github-bot
Copy link
Contributor

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

@cccclai @facebook-github-bot
Allow getting all backend names (pytorch#8520)
26f8fa8 
Summary:

Allow getting all backends name in both python and c++

Reviewed By: omerjerk

Differential Revision: D69691354
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot facebook-github-bot merged commit 0490b0e into pytorch:main Feb 21, 2025
47 of 50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mergennachin mergennachin mergennachin approved these changes

@GregoryComer GregoryComer GregoryComer left review comments

@dbort dbort dbort approved these changes

@larryliu0820 larryliu0820 larryliu0820 approved these changes

@tarun292 tarun292 tarun292 approved these changes

@JacobSzwejbka JacobSzwejbka Awaiting requested review from JacobSzwejbka JacobSzwejbka is a code owner

@lucylq lucylq Awaiting requested review from lucylq lucylq is a code owner

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported release notes: api Changes to public facing apis (any interfaces, pybinded runtime methods, etc.)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@cccclai @facebook-github-bot @larryliu0820 @GregoryComer @mergennachin @dbort @tarun292