Skip to content

Conversation

@JacobSzwejbka
Copy link
Contributor

Summary:
Cleaned up the existing pass and fixed a typing error (EP -> PassResult),

added another option in backend config to extract only mutable weights (training workflows will do this),

fixed the ordering of ET passes and added a warning not to add stuff after memory planning (this pass was actually fine but in general we like having the invariant that memory planning is last),

fixed the emitter to prioritize making it external vs mutable. Down the line we will need to support intermixing of mutable and non mutable in the same .ptd (memory regressions not correctness are the stakes), but no one needs that today so deferring.

Reviewed By: lucylq

Differential Revision: D68121580

@pytorch-bot
Copy link

pytorch-bot bot commented Jan 21, 2025

🔗 Helpful Links

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

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

❌ 1 New Failure

As of commit b67b193 with merge base 948fba6 (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 21, 2025
@facebook-github-bot
Copy link
Contributor

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

Summary:

Cleaned up the existing pass and fixed a typing error (EP -> PassResult),

added another option in backend config to extract only mutable weights (training workflows will do this),

fixed the ordering of ET passes and added a warning not to add stuff after memory planning (this pass was actually fine but in general we like having the invariant that memory planning is last),

fixed the emitter to prioritize making it external vs mutable. Down the line we will need to support intermixing of mutable and non mutable in the same .ptd (memory regressions not correctness are the stakes), but no one needs that today so deferring.

Reviewed By: lucylq

Differential Revision: D68121580
@JacobSzwejbka JacobSzwejbka added the release notes: api Changes to public facing apis (any interfaces, pybinded runtime methods, etc.) label Jan 21, 2025
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot facebook-github-bot merged commit 4655202 into pytorch:main Jan 21, 2025
45 of 47 checks passed
YIWENX14 pushed a commit that referenced this pull request Jan 28, 2025
Differential Revision: D68121580

Pull Request resolved: #7798
zonglinpeng pushed a commit to zonglinpeng/executorch that referenced this pull request Jan 30, 2025
Differential Revision: D68121580

Pull Request resolved: pytorch#7798
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: api Changes to public facing apis (any interfaces, pybinded runtime methods, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants