Skip to content
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.

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

remove PP tracer #555

Merged
merged 2 commits into from
Aug 22, 2024
Merged

remove PP tracer #555

merged 2 commits into from
Aug 22, 2024

Conversation

tianyu-l
Copy link
Contributor

@tianyu-l tianyu-l commented Aug 22, 2024

Stack from ghstack (oldest at bottom):

Discussed with @wconstab and @kwen2501 , it seems PP tracer has two limitations right now:

  1. It doesn't support init_weights, thus requiring a seed checkpoint to do init, which is something we probably will deprecate soon after we have the init support for manual splitting.
  2. It doesn't support mixed precision training, as the tracer requires that the model being traced be consistent with the model during forward.

We came to the conclusion to remove PP tracer for now, to have a clean version of torchtitan.

[ghstack-poisoned]
tianyu-l added a commit that referenced this pull request Aug 22, 2024
ghstack-source-id: 6e56baaa8c48c451b6dac1945310e62d310b19f7
Pull Request resolved: #555
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Aug 22, 2024
@tianyu-l tianyu-l requested review from wconstab and kwen2501 August 22, 2024 00:14
Discussed with wconstab and kwen2501 , it seems PP tracer has two limitations right now:
1. It doesn't support `init_weights`, thus requiring a seed checkpoint to do init, which is something we probably will deprecate soon after we have the init support for manual splitting.
2. It doesn't support mixed precision training, as the tracer requires that the model being traced be consistent with the model during forward.

We came to the conclusion to remove PP tracer for now, to have a clean version of torchtitan.

[ghstack-poisoned]
tianyu-l added a commit that referenced this pull request Aug 22, 2024
ghstack-source-id: ab6a7cec6ba4f4690f5834d22bc16d8d9f2bdba8
Pull Request resolved: #555
@tianyu-l tianyu-l merged commit 5ff55a0 into gh/tianyu-l/21/base Aug 22, 2024
5 checks passed
tianyu-l added a commit that referenced this pull request Aug 22, 2024
ghstack-source-id: ab6a7cec6ba4f4690f5834d22bc16d8d9f2bdba8
Pull Request resolved: #555
@tianyu-l tianyu-l deleted the gh/tianyu-l/21/head branch August 22, 2024 23:06
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 Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants