Release of MMPROJ and GGUF format files?

GJSea commented 5 months ago

Hi,

Bunny-Llama-3-8B-V is very impressive and is able to pick up far more details than anything that uses clip-vit-large-patch14-336!

As most of the community uses LLAMA.CPP to develop on, I was wondering if you might be able to publish the GGUF and MMPROJ pairing files for you model? It would get significant adoption if in this format.

Thx!

Isaachhh commented 5 months ago

Hi! We released GGUF here.

GJSea commented 5 months ago

Hi,

Thanks so much for the model files!

I might have found a bug in LLAVA.CPP, though I’m not sure if it is a data error in the positions tensor.

In clip.cpp -> clip_image_batch_encode, num_positions is calculated to be 730.
ggml_nbytes(positions) allocates positions_data buffer for 2,916 bytes.
However for (int i = 0; i < num_positions; i++) ==> 730 * sizeof(int) = 2,920 which over indexes the positions_data buffer size of 2916
free(positions_data); causes a heap corruption.

If I hard allocate positions_data to be 2,920 bytes, your model works wonderfully.

So I’m not sure:

If num_positions should be without the +1
If for (int i = 0; i < num_positions; i++) should be for (int i = 0; i < ggml_nbytes(positions) / sizeof(int); i++)
Or if there is a data error in ggml_graph_get_tensor(gf, "positions");

I’m happy to file a bug with llama.cpp if this isn’t a data error in the MMPROJ file.

Thanks! Gavin

From: Isaachhh @.> Sent: Sunday, May 5, 2024 11:17 PM To: BAAI-DCAI/Bunny @.> Cc: Gavin Jancke @.>; Author @.> Subject: Re: [BAAI-DCAI/Bunny] Release of MMPROJ and GGUF format files? (Issue #64)

Hi! We released GGUF herehttps://huggingface.co/BoyaWu10/Bunny-Llama-3-8B-V-gguf.

— Reply to this email directly, view it on GitHubhttps://github.com/BAAI-DCAI/Bunny/issues/64#issuecomment-2095264559, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AFBYJNGWSBUFUVFBIBJ4VTLZA4N4BAVCNFSM6AAAAABHCNQTLSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJVGI3DINJVHE. You are receiving this because you authored the thread.Message ID: @.**@.>>

Isaachhh commented 5 months ago

Hi, Gavin,

TL;DR num_positions should be without the +1

Actually, Bunny-Llama-3-8B-V is built upon SigLIP, while llama.cpp only supports CLIP now. We use the clip.cpp to load SigLIP weights temporarily now but we are not fully sure whether it's reasonable. And CLIP has a CLS token while SigLIP not, so num_positions should be without the +1.

However, although there are an out-of-bounds circumstance, I don't meet the heap corruption and the output is the same whether with the +1 or not. Thanks for pointing out that.

Isaachhh commented 4 months ago

This problem has been solved by https://github.com/ggerganov/llama.cpp/commit/d11afd665241c1b3910ab5f040d0216403019d87

Isaachhh commented 4 months ago

Close the issue for now if there's no further discussions. Feel free to reopen it if there's any other questions.

GJSea commented 4 months ago

Thanks so much, really appreciate your help!

From: Isaachhh @.> Sent: Sunday, June 2, 2024 5:44:30 AM To: BAAI-DCAI/Bunny @.> Cc: Gavin Jancke @.>; Author @.> Subject: Re: [BAAI-DCAI/Bunny] Release of MMPROJ and GGUF format files? (Issue #64)

Closed #64https://github.com/BAAI-DCAI/Bunny/issues/64 as completed.

— Reply to this email directly, view it on GitHubhttps://github.com/BAAI-DCAI/Bunny/issues/64#event-13011627868, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AFBYJNC5RSCEHIAYEOXT55DZFMHS5AVCNFSM6AAAAABHCNQTLSVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJTGAYTCNRSG44DMOA. You are receiving this because you authored the thread.Message ID: @.***>

BAAI-DCAI / Bunny

Release of MMPROJ and GGUF format files? #64