Add a default delegate time scale converter #5076
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/5076
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit d7c3001 with merge base 85410e4 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D62160650 |
|
This pull request was exported from Phabricator. Differential Revision: D62160650 |
Olivia-liu
added a commit
to Olivia-liu/executorch-1
that referenced
this pull request
Sep 4, 2024
Summary: Pull Request resolved: pytorch#5076 By default, the non-delegated events have a time scale converter, but delegated events do not, and this has caused confusion for users: [post](https://fb.workplace.com/groups/pytorch.edge.users/permalink/1562687731268181/), [Issue pytorch#4504](pytorch#4504). This diff gives a quick solution for the problem to unblock ET Beta. The rationale is that since for non-delegated ops, we have the default time scale: ``` source_time_scale: TimeScale = TimeScale.NS, target_time_scale: TimeScale = TimeScale.MS, ``` And the conversion rate is 10^6 (because 1 MS = 10^6 NS), so it makes sense to set a default converter function for delegated ops to be `input_time/(1000 * 1000)` as well. Similar converter functions have been used by [coreml](https://www.internalfb.com/code/fbsource/[76be64aab17c]/fbcode/executorch/examples/apple/coreml/scripts/inspector_utils.py?lines=259-262) and [vulkan](https://fburl.com/code/5cu6fx2b) already. The proper solution to write the convertion in etdump is tracked by T198369344 and T198369419 Differential Revision: D62160650
Olivia-liu
force-pushed
the
export-D62160650
branch
from
2be7433 to
412c435
Compare
|
This pull request was exported from Phabricator. Differential Revision: D62160650 |
Olivia-liu
force-pushed
the
export-D62160650
branch
from
412c435 to
68997d8
Compare
Olivia-liu
added a commit
to Olivia-liu/executorch-1
that referenced
this pull request
Sep 4, 2024
Summary: Pull Request resolved: pytorch#5076 By default, the non-delegated events have a time scale converter, but delegated events do not, and this has caused confusion for users: [post](https://fb.workplace.com/groups/pytorch.edge.users/permalink/1562687731268181/), [Issue pytorch#4504](pytorch#4504). This diff gives a quick solution for the problem to unblock ET Beta. When no `delegate_time_scale_converter` is provided by the user, just use the same converting ratio as the non-delegated events. The proper solution to write the convertion in etdump is tracked by T198369344 and T198369419 Differential Revision: D62160650
dbort
approved these changes
Sep 4, 2024
|
This pull request was exported from Phabricator. Differential Revision: D62160650 |
Olivia-liu
added a commit
to Olivia-liu/executorch-1
that referenced
this pull request
Sep 9, 2024
Summary: Pull Request resolved: pytorch#5076 By default, the non-delegated events have a time scale converter, but delegated events do not, and this has caused confusion for users: [post](https://fb.workplace.com/groups/pytorch.edge.users/permalink/1562687731268181/), [Issue pytorch#4504](pytorch#4504). This diff gives a quick solution for the problem to unblock ET Beta. When no `delegate_time_scale_converter` is provided by the user, just use the same converting ratio as the non-delegated events. The proper solution to write the convertion in etdump is tracked by T198369344 and T198369419 Reviewed By: dbort Differential Revision: D62160650
Olivia-liu
force-pushed
the
export-D62160650
branch
from
68997d8 to
bf55707
Compare
|
This pull request was exported from Phabricator. Differential Revision: D62160650 |
Olivia-liu
added a commit
to Olivia-liu/executorch-1
that referenced
this pull request
Sep 9, 2024
Summary: Pull Request resolved: pytorch#5076 By default, the non-delegated events have a time scale converter, but delegated events do not, and this has caused confusion for users: [post](https://fb.workplace.com/groups/pytorch.edge.users/permalink/1562687731268181/), [Issue pytorch#4504](pytorch#4504). This diff gives a quick solution for the problem to unblock ET Beta. When no `delegate_time_scale_converter` is provided by the user, just use the same converting ratio as the non-delegated events. The proper solution to write the convertion in etdump is tracked by T198369344 and T198369419 Reviewed By: dbort Differential Revision: D62160650
Olivia-liu
force-pushed
the
export-D62160650
branch
from
bf55707 to
1bde121
Compare
Summary: Pull Request resolved: pytorch#5076 By default, the non-delegated events have a time scale converter, but delegated events do not, and this has caused confusion for users: [post](https://fb.workplace.com/groups/pytorch.edge.users/permalink/1562687731268181/), [Issue pytorch#4504](pytorch#4504). This diff gives a quick solution for the problem to unblock ET Beta. When no `delegate_time_scale_converter` is provided by the user, just use the same converting ratio as the non-delegated events. The proper solution to write the convertion in etdump is tracked by T198369344 and T198369419 Reviewed By: dbort Differential Revision: D62160650
Olivia-liu
force-pushed
the
export-D62160650
branch
from
1bde121 to
d7c3001
Compare
|
This pull request was exported from Phabricator. Differential Revision: D62160650 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Because the unit of delegate ops are different from non-delegates, this has caused confusion for users: post, Issue #4504.
This diff gives a quick solution for the problem to unblock Beta. The rationale is that since for non-delegated ops, we have the default time scale:
And the conversion rate is 10^6 (because 1 MS = 10^6 NS), so it makes sense to set a default converter function for delegated ops to be
input_time/(1000 * 1000)as well. Similar converter functions have been used by coreml and vulkan already.The proper solution to write the convertion in etdump is tracked by T198369344 and T198369419
Differential Revision: D62160650