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

Support for LLaVA #88

Merged
merged 19 commits into from
Jul 30, 2024
Merged

Support for LLaVA #88

merged 19 commits into from
Jul 30, 2024

Conversation

varshith15
Copy link
Contributor

@varshith15 varshith15 commented Jul 26, 2024

  • Single Machine
  • Sharding
  • Generate function integration
  • Clean up code (deduplication)

@AlexCheema
Copy link
Contributor

Looks good so far!

Assigned you on the bounty https://docs.google.com/spreadsheets/d/1cTCpTIp48UnnIvHeLEUNg1iMy_Q6lRybgECSFCoVJpE/edit

@varshith15
Copy link
Contributor Author

varshith15 commented Jul 27, 2024

hey @AlexCheema

  • vision model is fully on the first shard, language model is multi sharded
  • integrated llava into generate function as well
  • only the main.py integration is left which i feel like should be a seperate PR (could be breaking changes)
  • There are no breaking changes to the current flow.
    Please review and merge

@varshith15 varshith15 marked this pull request as ready for review July 27, 2024 19:20
@AlexCheema
Copy link
Contributor

We should merge #87 before this as there's some relevant changes.
@varshith15 you can merge that branch already if you want and then I will merge yours as soon as the other is merged.

@AlexCheema
Copy link
Contributor

With changes from #87, we should try to re-use as much as possible from mlx_lm, there's a lot of copied code when it should only be necessary to copy 1 or 2 classes from there and modify them.

@AlexCheema
Copy link
Contributor

Is the test_sharded_llava.py test passing for you?
I thought we'd need to load an mlx-converted model, but it looks like this is the original ordinary model from HF llava-hf/llava-1.5-7b-hf

@AlexCheema
Copy link
Contributor

Is the test_sharded_llava.py test passing for you? I thought we'd need to load an mlx-converted model, but it looks like this is the original ordinary model from HF llava-hf/llava-1.5-7b-hf

NVM this works fine :)

if self.shard.start_layer <= i <= self.shard.end_layer:
self.layers.append(TransformerBlock(config=config))
else:
self.layers.append(IdentityBlock())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this isnt needed right @AlexCheema ?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is with the new convention, I think

@varshith15
Copy link
Contributor Author

also thanks thanks for pushing the other changes @AlexCheema , they make a lot of sense

@AlexCheema
Copy link
Contributor

AlexCheema commented Jul 28, 2024

It would be great to hook this up to chatgpt_api.py do you want to give that a go @varshith15 ? i.e. allow the user to give an image as input, similar to how chatgpt vision api works.

@varshith15
Copy link
Contributor Author

@AlexCheema done PRM
the logic to use the pixel_values only in the first pass fits in perfectly with the send_prompt, send_tensor idea (really cool idea)
i've verified, its backward compatible as well
only change left is the UI one, I am little occupied so can't pick that up rn

@AlexCheema
Copy link
Contributor

@AlexCheema done PRM the logic to use the pixel_values only in the first pass fits in perfectly with the send_prompt, send_tensor idea (really cool idea) i've verified, its backward compatible as well only change left is the UI one, I am little occupied so can't pick that up rn

Woah! Incredible work. You even figured out how to generate the grpc service. I’m catching a flight now but will be able to look properly once I arrive.

I can fix up a small UI.

email me at [email protected] for the bounty.

really awesome work, you rock @varshith15

@AlexCheema AlexCheema merged commit 0ec77e1 into exo-explore:main Jul 30, 2024
3 checks passed
@AlexCheema
Copy link
Contributor

Changed a few things, added support for image upload to tiny chat.
Awesome work @varshith15 please email me [email protected] for the bounty reward!

dan-online pushed a commit to bytebolt-media/exo that referenced this pull request Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants