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

llava example fix for wide images #5493

Closed

Conversation

jpohhhh
Copy link

@jpohhhh jpohhhh commented Feb 14, 2024

On wide images, but seemingly not tall, there was a crash on memcpy.

Investigation showed the number of embeddings differed from num patches. (i.e. a print statement showed num_images != image_embd_v.size() for a wide aspect ratio image)

This is the most minimal change that resolves the crash, reviewers familiar with clip may identify
a better fix based on grid_shape.

On wide images, but seemingly not tall, there was a crash on memcpy.

Investigation showed the number of embeddings differed from num patches.
(i.e. a print statement showed num_images != image_embd_v.size() for a wide aspect ratio image)

This is the most minimal change that resolves the crash,
reviewers familiar with clip may identify
a better fix based on grid_shape.
@jpohhhh
Copy link
Author

jpohhhh commented Feb 14, 2024

cc @cmp-nct

@jpohhhh jpohhhh mentioned this pull request Feb 14, 2024
@cmp-nct
Copy link
Contributor

cmp-nct commented Feb 14, 2024

I'm looking into it
One of the largest changes at the end was to remove vector passing of the image embeddings, replace it with different logic and memory management.
It's likely related to that

@cmp-nct
Copy link
Contributor

cmp-nct commented Feb 14, 2024

Solved here: #5495

@jpohhhh
Copy link
Author

jpohhhh commented Feb 14, 2024

Closing

@jpohhhh jpohhhh closed this Feb 14, 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