Skip to content

Conversation

@junpi3
Copy link
Contributor

@junpi3 junpi3 commented Jan 17, 2025

Stack from ghstack (oldest at bottom):

Issue

In the ET-SDK, we assign an event_name to each operation. In ET-VK, we compose a unique event_name using the node_id. The node_id exists for every OperatorCall but not for input/output with nchw_to_image/image_to_nchw. Those cases collapse into node_id == 0 which means all nchw_to_image had the same event_name and hence only one was stored. The same reasoning results in storage of only one image_to_nchw.

Solution

Ignore the serialized node_id and use the operation's prepack_node/execute_node vector index.

TODO: Determine if we can remove the serialized node_id, or whether this should be fixed differently and still reference it.

Differential Revision: D68344534

## Issue
In the ET-SDK, we assign an `event_name` to each operation. In ET-VK, we compose a unique `event_name` using the `node_id`. The `node_id` exists for every `OperatorCall` but not for input/output with `nchw_to_image`/`image_to_nchw`. Those cases collapse into `node_id == 0` which means all `nchw_to_image` had the same `event_name` and hence only one was stored. The same reasoning results in storage of only one `image_to_nchw`.

## Solution
Ignore the serialized `node_id` and use the operation's `prepack_node`/`execute_node` vector index.

TODO: Determine if we can remove the serialized `node_id`, or whether this should be fixed differently and still reference it.

Differential Revision: [D68344534](https://our.internmc.facebook.com/intern/diff/D68344534/)

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Jan 17, 2025

🔗 Helpful Links

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

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

❌ 1 New Failure

As of commit 1e86abe with merge base 8611aaa (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 Jan 17, 2025
junpi3 pushed a commit that referenced this pull request Jan 17, 2025
## Issue
In the ET-SDK, we assign an `event_name` to each operation. In ET-VK, we compose a unique `event_name` using the `node_id`. The `node_id` exists for every `OperatorCall` but not for input/output with `nchw_to_image`/`image_to_nchw`. Those cases collapse into `node_id == 0` which means all `nchw_to_image` had the same `event_name` and hence only one was stored. The same reasoning results in storage of only one `image_to_nchw`.

## Solution
Ignore the serialized `node_id` and use the operation's `prepack_node`/`execute_node` vector index.

TODO: Determine if we can remove the serialized `node_id`, or whether this should be fixed differently and still reference it.

Differential Revision: [D68344534](https://our.internmc.facebook.com/intern/diff/D68344534/)

ghstack-source-id: 261951242
Pull Request resolved: #7738
@facebook-github-bot
Copy link
Contributor

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

@junpi3 junpi3 added the release notes: vulkan Changes to the Vulkan backend delegate label Jan 17, 2025
## Issue
In the ET-SDK, we assign an `event_name` to each operation. In ET-VK, we compose a unique `event_name` using the `node_id`. The `node_id` exists for every `OperatorCall` but not for input/output with `nchw_to_image`/`image_to_nchw`. Those cases collapse into `node_id == 0` which means all `nchw_to_image` had the same `event_name` and hence only one was stored. The same reasoning results in storage of only one `image_to_nchw`.

## Solution
Ignore the serialized `node_id` and use the operation's `prepack_node`/`execute_node` vector index.

TODO: Determine if we can remove the serialized `node_id`, or whether this should be fixed differently and still reference it.

Differential Revision: [D68344534](https://our.internmc.facebook.com/intern/diff/D68344534/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

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

## Issue
In the ET-SDK, we assign an `event_name` to each operation. In ET-VK, we compose a unique `event_name` using the `node_id`. The `node_id` exists for every `OperatorCall` but not for input/output with `nchw_to_image`/`image_to_nchw`. Those cases collapse into `node_id == 0` which means all `nchw_to_image` had the same `event_name` and hence only one was stored. The same reasoning results in storage of only one `image_to_nchw`.

## Solution
Ignore the serialized `node_id` and use the operation's `prepack_node`/`execute_node` vector index.

TODO: Determine if we can remove the serialized `node_id`, or whether this should be fixed differently and still reference it.

Differential Revision: [D68344534](https://our.internmc.facebook.com/intern/diff/D68344534/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot facebook-github-bot merged commit 0bbdabe into gh/jorgep31415/129/base Jan 21, 2025
45 of 47 checks passed
@facebook-github-bot facebook-github-bot deleted the gh/jorgep31415/129/head branch January 21, 2025 19:48
kirklandsign pushed a commit that referenced this pull request Jan 22, 2025
Pull Request resolved: #7738

## Issue
In the ET-SDK, we assign an `event_name` to each operation. In ET-VK, we compose a unique `event_name` using the `node_id`. The `node_id` exists for every `OperatorCall` but not for input/output with `nchw_to_image`/`image_to_nchw`. Those cases collapse into `node_id == 0` which means all `nchw_to_image` had the same `event_name` and hence only one was stored. The same reasoning results in storage of only one `image_to_nchw`.

## Solution
Ignore the serialized `node_id` and use the operation's `prepack_node`/`execute_node` vector index.

TODO: Determine if we can remove the serialized `node_id`, or whether this should be fixed differently and still reference it.

ghstack-source-id: 262309622
@exported-using-ghexport

Differential Revision: [D68344534](https://our.internmc.facebook.com/intern/diff/D68344534/)

Co-authored-by: jorgep31415 <[email protected]>
YIWENX14 pushed a commit that referenced this pull request Jan 28, 2025
Pull Request resolved: #7738

## Issue
In the ET-SDK, we assign an `event_name` to each operation. In ET-VK, we compose a unique `event_name` using the `node_id`. The `node_id` exists for every `OperatorCall` but not for input/output with `nchw_to_image`/`image_to_nchw`. Those cases collapse into `node_id == 0` which means all `nchw_to_image` had the same `event_name` and hence only one was stored. The same reasoning results in storage of only one `image_to_nchw`.

## Solution
Ignore the serialized `node_id` and use the operation's `prepack_node`/`execute_node` vector index.

TODO: Determine if we can remove the serialized `node_id`, or whether this should be fixed differently and still reference it.

ghstack-source-id: 262309622
@exported-using-ghexport

Differential Revision: [D68344534](https://our.internmc.facebook.com/intern/diff/D68344534/)

Co-authored-by: jorgep31415 <[email protected]>
zonglinpeng pushed a commit to zonglinpeng/executorch that referenced this pull request Jan 30, 2025
Pull Request resolved: pytorch#7738

## Issue
In the ET-SDK, we assign an `event_name` to each operation. In ET-VK, we compose a unique `event_name` using the `node_id`. The `node_id` exists for every `OperatorCall` but not for input/output with `nchw_to_image`/`image_to_nchw`. Those cases collapse into `node_id == 0` which means all `nchw_to_image` had the same `event_name` and hence only one was stored. The same reasoning results in storage of only one `image_to_nchw`.

## Solution
Ignore the serialized `node_id` and use the operation's `prepack_node`/`execute_node` vector index.

TODO: Determine if we can remove the serialized `node_id`, or whether this should be fixed differently and still reference it.

ghstack-source-id: 262309622
@exported-using-ghexport

Differential Revision: [D68344534](https://our.internmc.facebook.com/intern/diff/D68344534/)

Co-authored-by: jorgep31415 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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: vulkan Changes to the Vulkan backend delegate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants