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

[Multimodal model] Add the decoder of the multimodal model #626

Merged
merged 3 commits into from
Oct 25, 2024

Conversation

fduwjj
Copy link
Contributor

@fduwjj fduwjj commented Oct 18, 2024

After #589, we are adding decoder model to torchtitan with simple unit test.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 18, 2024
@fduwjj fduwjj mentioned this pull request Oct 18, 2024
Copy link
Contributor

@tianyu-l tianyu-l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed offline, let's create separate files (in the folder) for encoder, decoder, and shared parts. This would make code more readable than a 1.5k LoC monolithic file.

I had a similar suggestion for ModelArgs, since it seems to be shared by encoder and decoder.

@fduwjj fduwjj requested review from awgu and tianyu-l October 22, 2024 16:33
Copy link
Contributor

@tianyu-l tianyu-l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Please update the ModelArgs before merge.

Copy link
Contributor

@kwen2501 kwen2501 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

@fduwjj fduwjj merged commit ccfc02b into main Oct 25, 2024
5 checks passed
@fduwjj fduwjj deleted the vision_decoder branch October 25, 2024 03:34
mori360 pushed a commit to mori360/torchtitan that referenced this pull request Nov 26, 2024
After pytorch#589, we are adding
decoder model to torchtitan with simple unit test.
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.

5 participants