Skip to content

Conversation

skrtskrtfb
Copy link
Contributor

Summary:
Executorch memory planning currently accepts a string identifier to represent the desired algorithm.

However, this makes it difficult to pass custom arguments to write more customized memory planning algorithms. This change allows users to pass the full memory planning function as an argument as opposed to just the string identifier.

Core changes in:

  • fbcode/executorch/exir/passes/memory_planning_pass.py
  • fbcode/executorch/exir/tests/test_memory_planning.py

Remaining changes are just to enforce compliance with new API at all call sites in codebase

NOTE: A less intrusive change could be to allow argument to be either string or entire custom functions. I opted for just passing only functions to simplify and avoid confusion

Reviewed By: zonglinpengmeta, mcremon-meta

Differential Revision: D60433641

Copy link

pytorch-bot bot commented Aug 15, 2024

🔗 Helpful Links

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

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

✅ No Failures

As of commit 93cb8d7 with merge base 26375cc (image):
💚 Looks good so far! There are no failures yet. 💚

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 Aug 15, 2024
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

skrtskrtfb pushed a commit to skrtskrtfb/executorch that referenced this pull request Aug 15, 2024
…posed to string name (pytorch#4727)

Summary:
Pull Request resolved: pytorch#4727

Executorch memory planning currently accepts a string identifier to represent the desired algorithm.

However, this makes it difficult to pass custom arguments to write more customized memory planning algorithms. This change allows users to pass the full memory planning function as an argument as opposed to just the string identifier.

Core changes in:
- fbcode/executorch/exir/passes/memory_planning_pass.py
- fbcode/executorch/exir/tests/test_memory_planning.py

Remaining changes are just to enforce compliance with new API at all call sites in codebase

NOTE: A less intrusive change could be to allow argument to be either string or entire custom functions. I opted for just passing only functions to simplify and avoid confusion

Reviewed By: zonglinpengmeta, mcremon-meta

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

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

skrtskrtfb pushed a commit to skrtskrtfb/executorch that referenced this pull request Aug 15, 2024
…posed to string name (pytorch#4727)

Summary:
Pull Request resolved: pytorch#4727

Executorch memory planning currently accepts a string identifier to represent the desired algorithm.

However, this makes it difficult to pass custom arguments to write more customized memory planning algorithms. This change allows users to pass the full memory planning function as an argument as opposed to just the string identifier.

Core changes in:
- fbcode/executorch/exir/passes/memory_planning_pass.py
- fbcode/executorch/exir/tests/test_memory_planning.py

Remaining changes are just to enforce compliance with new API at all call sites in codebase

NOTE: A less intrusive change could be to allow argument to be either string or entire custom functions. I opted for just passing only functions to simplify and avoid confusion

Reviewed By: zonglinpengmeta, mcremon-meta

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

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

skrtskrtfb pushed a commit to skrtskrtfb/executorch that referenced this pull request Sep 13, 2024
…posed to string name (pytorch#4727)

Summary:
Pull Request resolved: pytorch#4727

Executorch memory planning currently accepts a string identifier to represent the desired algorithm.

However, this makes it difficult to pass custom arguments to write more customized memory planning algorithms. This change allows users to pass the full memory planning function as an argument as opposed to just the string identifier.

Core changes in:
- fbcode/executorch/exir/passes/memory_planning_pass.py
- fbcode/executorch/exir/tests/test_memory_planning.py

Remaining changes are just to enforce compliance with new API at all call sites in codebase

NOTE: A less intrusive change could be to allow argument to be either string or entire custom functions. I opted for just passing only functions to simplify and avoid confusion

Reviewed By: zonglinpeng, mcremon-meta

Differential Revision: D60433641
…posed to string name (pytorch#4727)

Summary:
Pull Request resolved: pytorch#4727

Executorch memory planning currently accepts a string identifier to represent the desired algorithm.

However, this makes it difficult to pass custom arguments to write more customized memory planning algorithms. This change allows users to pass the full memory planning function as an argument as opposed to just the string identifier.

Core changes in:
- fbcode/executorch/exir/passes/memory_planning_pass.py
- fbcode/executorch/exir/tests/test_memory_planning.py

Remaining changes are just to enforce compliance with new API at all call sites in codebase

NOTE: A less intrusive change could be to allow argument to be either string or entire custom functions. I opted for just passing only functions to simplify and avoid confusion

Reviewed By: zonglinpeng, mcremon-meta

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

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

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 618466e.

@dbort
Copy link
Contributor

dbort commented Oct 10, 2024

@pytorchbot cherry-pick --onto release/0.4 -c fixnewfeature

pytorchbot pushed a commit that referenced this pull request Oct 10, 2024
…posed to string name (#4727)

Summary:
Pull Request resolved: #4727

Executorch memory planning currently accepts a string identifier to represent the desired algorithm.

However, this makes it difficult to pass custom arguments to write more customized memory planning algorithms. This change allows users to pass the full memory planning function as an argument as opposed to just the string identifier.

Core changes in:
- fbcode/executorch/exir/passes/memory_planning_pass.py
- fbcode/executorch/exir/tests/test_memory_planning.py

Remaining changes are just to enforce compliance with new API at all call sites in codebase

NOTE: A less intrusive change could be to allow argument to be either string or entire custom functions. I opted for just passing only functions to simplify and avoid confusion

Reviewed By: zonglinpeng, hsharma35, mcremon-meta

Differential Revision: D60433641

fbshipit-source-id: 0fe3677b7c3f4c3763cb1b4fe6d28ef814f2ecf9
(cherry picked from commit 618466e)
@pytorchbot
Copy link
Collaborator

Cherry picking #4727

The cherry pick PR is at #6130 and it is recommended to link a fixnewfeature cherry pick PR with an issue. The following tracker issues are updated:

Details for Dev Infra team Raised by workflow job

dbort pushed a commit that referenced this pull request Oct 10, 2024
…posed to string name (#4727)

Summary:
Pull Request resolved: #4727

Executorch memory planning currently accepts a string identifier to represent the desired algorithm.

However, this makes it difficult to pass custom arguments to write more customized memory planning algorithms. This change allows users to pass the full memory planning function as an argument as opposed to just the string identifier.

Core changes in:
- fbcode/executorch/exir/passes/memory_planning_pass.py
- fbcode/executorch/exir/tests/test_memory_planning.py

Remaining changes are just to enforce compliance with new API at all call sites in codebase

NOTE: A less intrusive change could be to allow argument to be either string or entire custom functions. I opted for just passing only functions to simplify and avoid confusion

Reviewed By: zonglinpeng, hsharma35, mcremon-meta

Differential Revision: D60433641

fbshipit-source-id: 0fe3677b7c3f4c3763cb1b4fe6d28ef814f2ecf9
(cherry picked from commit 618466e)
dbort pushed a commit that referenced this pull request Oct 11, 2024
…posed to string name (#6130)

Change memory planning API to accept full algorithm as argument as opposed to string name (#4727)

Summary:
Pull Request resolved: #4727

Executorch memory planning currently accepts a string identifier to represent the desired algorithm.

However, this makes it difficult to pass custom arguments to write more customized memory planning algorithms. This change allows users to pass the full memory planning function as an argument as opposed to just the string identifier.

Core changes in:
- fbcode/executorch/exir/passes/memory_planning_pass.py
- fbcode/executorch/exir/tests/test_memory_planning.py

Remaining changes are just to enforce compliance with new API at all call sites in codebase

NOTE: A less intrusive change could be to allow argument to be either string or entire custom functions. I opted for just passing only functions to simplify and avoid confusion

Reviewed By: zonglinpeng, hsharma35, mcremon-meta

Differential Revision: D60433641

fbshipit-source-id: 0fe3677b7c3f4c3763cb1b4fe6d28ef814f2ecf9
(cherry picked from commit 618466e)

Co-authored-by: Eashan Garg <[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 Merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants