Repository navigation
clip related crash, segmentation fault #837
Description
Activity
does
--type f32fix it?it doesn't have enough memory to test
--type f32but f16 worked fine. also q4_0 and q4_1 works too. problem is q4_k.Maybe it's somehow related to that?
https://github.com/leejet/stable-diffusion.cpp/blob/master/docs/lora.md#support-matrixDoes
--clip-on-cpuwork as a workaround to avoid the crash?Passing
--embd-dir(pointing to any directory) should work around the crash, too.both "--clip-on-cpu" and "--embd-dir" with empty directory avoids crash. also i don't use any lora.
I know you're not using loras, but my guess is that it's crashing for the same underlying reason lora crashes with some types on cuda (with get_rows op).
I was able to reproduce it here on Vulkan, with an SD1.5 model converted to Q4_K. Probably for the same reason: https://github.com/ggml-org/ggml/blob/5fdc78fff274094e2a1b155928131983362d8a71/src/ggml-vulkan/ggml-vulkan.cpp#L11940
I know you're not using loras, but my guess is that it's crashing for the same underlying reason lora crashes with some types on cuda (with get_rows op).
i see. it looks like previous versions always used f32 for clip but this changed after that commit with
force_clip_f32check. that explains why it causes problems when embd-dir is not provided.stable-diffusion.cpp/conditioner.hpp
Line 66 in 79426d5
solution?- providing empty embd-dir
- keeping models with non-K quantization
- using clip on cpu
- implementing guardrail to pick nearest supported quantization in the case of unsupported quantization (?)
i'm not sure what to do about
crash by defaultorcrash if not supplied with an argumentbehaviour though.Oh, it definitely shouldn't crash 🙂 Giving a bit of context: passing an embeddings directory turns on the
embd_dir.size() > 0check, which replicates at runtime the reason for the upcast to f32 in the first place ( fbd42b6 ). But it turns out that change helps to avoid other quant issues, like this one.@stduhpf , I guess the more appropriate fix would be calling
ggml_backend_supports_opto check for get_rows availability?Reacted by stduhpf@wbruna yes, but the backend isn't passed to init_params(), so it will be a bit annoying to implement there.
Checking during the forward pass could work, but I'm not sure what you could do instead if get_rows is not supported.
Reacted by Wagner BrunaI think it's best to keep
token_embedding.weightin f16 or f32 without quantization. In fact, it's generally not recommended to quantizetoken_embedding.weight, as quantization may severely affect the model's accuracy.Reacted by stduhpf and kuesji koesnuI think it's best to keep
token_embedding.weightin f16 or f32 without quantization. In fact, it's generally not recommended to quantizetoken_embedding.weight, as quantization may severely affect the model's accuracy.On the other hand, token_embedding.weight can make up a significant amount of the total parameters of the model. If I remember correctly, llama.cpp quantizes it to Q6_K wtype for Q4_0 ftype, and runs the get_rows on CPU.
it seems like 48956ff introduced a bug and it causes crash on my laptop.
i tried to compile latest release but it was crashing. i was able the trace the issue back to that commit.
let me know if anything else is needed. i'm going to stay at ddc4a18 for now.
output of execution with -v arg