Skip to content

[ET-VK][ez] Fix partitioner logic #12196

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
Merged
SS-JIA merged 5 commits into from
Jul 8, 2025
Merged

[ET-VK][ez] Fix partitioner logic #12196

SS-JIA merged 5 commits into from
Jul 8, 2025

Conversation

@SS-JIA
Copy link
Contributor

@SS-JIA SS-JIA commented Jul 3, 2025

Summary:

Changes

  • In partitioner, check if buffer storage can be used only after it has been determined that no valid texture layouts are available

Context

Currently, the logic in the vulkan partitioner is incorrect.

  1. First, it checks what texture layouts may be used to represent the tensors involved in the computation
  2. If no texture layouts are available, it checks if buffer support is available and the tensors are small enough to be within Vulkan buffer limits
  3. Then, it checks if all valid texture layouts are supported by the op.

This introduces a bug in situations where 3 fails, but 2 would pass. However, 2 is not checked due to the way the logic is structured.

The fix is to switch the order of 2 and 3.

Test Plan:

Test Plan

Manual verification + CI

@SS-JIA
Update
5091b12 
[ghstack-poisoned]
@SS-JIA
Copy link
Contributor Author

SS-JIA commented Jul 3, 2025 •
edited
Loading

Stack from ghstack (oldest at bottom):

@pytorch-bot
Copy link

pytorch-bot bot commented Jul 3, 2025 •
edited
Loading

🔗 Helpful Links

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

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

⏳ No Failures, 9 Pending

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

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

SS-JIA added a commit that referenced this pull request Jul 3, 2025
@SS-JIA
[ET-VK][ez] Fix partitioner logic
c9235fb 
Summary:
## Changes

* In partitioner, check if buffer storage can be used only after it has been determined that no valid texture layouts are available

## Context

Currently, the logic in the vulkan partitioner is incorrect.

1. First, it checks what texture layouts may be used to represent the tensors involved in the computation
2. If no texture layouts are available, it checks if buffer support is available and the tensors are small enough to be within Vulkan buffer limits
3. Then, it checks if all valid texture layouts are supported by the op.

This introduces a bug in situations where 3 fails, but 2 would pass. However, 2 is not checked due to the way the logic is structured.

The fix is to switch the order of 2 and 3.


Test Plan:
## Test Plan

Manual verification + CI

ghstack-source-id: 9ccc526
ghstack-comment-id: 3032442697
Pull Request resolved: #12196
@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 Jul 3, 2025
@github-actions
Copy link

github-actions bot commented Jul 3, 2025

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

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

@SS-JIA
Update
aaeca67 
[ghstack-poisoned]
SS-JIA added a commit that referenced this pull request Jul 3, 2025
@SS-JIA
[ET-VK][ez] Fix partitioner logic
863cc9c 
Summary:
## Changes

* In partitioner, check if buffer storage can be used only after it has been determined that no valid texture layouts are available

## Context

Currently, the logic in the vulkan partitioner is incorrect.

1. First, it checks what texture layouts may be used to represent the tensors involved in the computation
2. If no texture layouts are available, it checks if buffer support is available and the tensors are small enough to be within Vulkan buffer limits
3. Then, it checks if all valid texture layouts are supported by the op.

This introduces a bug in situations where 3 fails, but 2 would pass. However, 2 is not checked due to the way the logic is structured.

The fix is to switch the order of 2 and 3.


Test Plan:
## Test Plan

Manual verification + CI

ghstack-source-id: 5a70bfd
ghstack-comment-id: 3032442697
Pull Request resolved: #12196
@SS-JIA
Update
606910f 
[ghstack-poisoned]
SS-JIA added a commit that referenced this pull request Jul 3, 2025
@SS-JIA
[ET-VK][ez] Fix partitioner logic
95a7177 
Summary:
## Changes

* In partitioner, check if buffer storage can be used only after it has been determined that no valid texture layouts are available

## Context

Currently, the logic in the vulkan partitioner is incorrect.

1. First, it checks what texture layouts may be used to represent the tensors involved in the computation
2. If no texture layouts are available, it checks if buffer support is available and the tensors are small enough to be within Vulkan buffer limits
3. Then, it checks if all valid texture layouts are supported by the op.

This introduces a bug in situations where 3 fails, but 2 would pass. However, 2 is not checked due to the way the logic is structured.

The fix is to switch the order of 2 and 3.


Test Plan:
## Test Plan

Manual verification + CI

ghstack-source-id: 229e617
ghstack-comment-id: 3032442697
Pull Request resolved: #12196
@SS-JIA
Update
1ca5d86 
[ghstack-poisoned]
SS-JIA added a commit that referenced this pull request Jul 8, 2025
@SS-JIA
[ET-VK][ez] Fix partitioner logic
de146c4 
Summary:
## Changes

* In partitioner, check if buffer storage can be used only after it has been determined that no valid texture layouts are available

## Context

Currently, the logic in the vulkan partitioner is incorrect.

1. First, it checks what texture layouts may be used to represent the tensors involved in the computation
2. If no texture layouts are available, it checks if buffer support is available and the tensors are small enough to be within Vulkan buffer limits
3. Then, it checks if all valid texture layouts are supported by the op.

This introduces a bug in situations where 3 fails, but 2 would pass. However, 2 is not checked due to the way the logic is structured.

The fix is to switch the order of 2 and 3.


Test Plan:
## Test Plan

Manual verification + CI

ghstack-source-id: 049da62
ghstack-comment-id: 3032442697
Pull Request resolved: #12196
@SS-JIA
Update
bd76973 
[ghstack-poisoned]
SS-JIA added a commit that referenced this pull request Jul 8, 2025
@SS-JIA
[ET-VK][ez] Fix partitioner logic
ba938c5 
Summary:
## Changes

* In partitioner, check if buffer storage can be used only after it has been determined that no valid texture layouts are available

## Context

Currently, the logic in the vulkan partitioner is incorrect.

1. First, it checks what texture layouts may be used to represent the tensors involved in the computation
2. If no texture layouts are available, it checks if buffer support is available and the tensors are small enough to be within Vulkan buffer limits
3. Then, it checks if all valid texture layouts are supported by the op.

This introduces a bug in situations where 3 fails, but 2 would pass. However, 2 is not checked due to the way the logic is structured.

The fix is to switch the order of 2 and 3.


Test Plan:
## Test Plan

Manual verification + CI

ghstack-source-id: ca0725f
ghstack-comment-id: 3032442697
Pull Request resolved: #12196
@SS-JIA SS-JIA merged commit a022db0 into main Jul 8, 2025
96 of 97 checks passed
@SS-JIA SS-JIA deleted the gh/SS-JIA/252/head branch July 8, 2025 14:35
Tanish2101 pushed a commit to Tanish2101/executorch that referenced this pull request Jul 9, 2025
@SS-JIA @Tanish2101
[ET-VK][ez] Fix partitioner logic (pytorch#12196)
3e04a72 
Summary:
## Changes

* In partitioner, check if buffer storage can be used only after it has
been determined that no valid texture layouts are available

## Context

Currently, the logic in the vulkan partitioner is incorrect.

1. First, it checks what texture layouts may be used to represent the
tensors involved in the computation
2. If no texture layouts are available, it checks if buffer support is
available and the tensors are small enough to be within Vulkan buffer
limits
3. Then, it checks if all valid texture layouts are supported by the op.

This introduces a bug in situations where 3 fails, but 2 would pass.
However, 2 is not checked due to the way the logic is structured.

The fix is to switch the order of 2 and 3.


Test Plan:
## Test Plan

Manual verification + CI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@junpi3 junpi3 junpi3 approved these changes

@JacobSzwejbka JacobSzwejbka Awaiting requested review from JacobSzwejbka

@larryliu0820 larryliu0820 Awaiting requested review from larryliu0820

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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@SS-JIA @junpi3 @facebook-github-bot