Skip to content

add progress callback, supress pretty_progress #170

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

Merged
merged 9 commits into from
Mar 2, 2024

Conversation

fszontagh
Copy link
Contributor

No description provided.

@leejet
Copy link
Owner

leejet commented Feb 24, 2024

Is there a specific reason to increase the size of many graphs and some reserved buffer sizes by 2 to 4 times? Additionally, ggml_n_dims_t seems to serve no purpose, as ggml_n_dims performs the same function. Passing n_dim directly into the creation of ggml_tensor from tensors_storage does not seem to cause any issues, as the calculation of ggml_tensor dimensions does not directly use the passed n_dim but instead calculates it through ggml_n_dims.

@fszontagh
Copy link
Contributor Author

Is there a specific reason to increase the size of many graphs and some reserved buffer sizes by 2 to 4 times? Additionally, ggml_n_dims_t seems to serve no purpose, as ggml_n_dims performs the same function. Passing n_dim directly into the creation of ggml_tensor from tensors_storage does not seem to cause any issues, as the calculation of ggml_tensor dimensions does not directly use the passed n_dim but instead calculates it through ggml_n_dims.

There is it: #178

But not just with the controlnet models. These errors is happened with larger file sized lora models too. I went through over my "lora collection", and tested some lora models. I modified these params while larger loras started to working.

ggml_n_dims_t seems to serve no purpose, as ggml_n_dims

Sorry, i don't remember what was the problem there. But the new method is required, because the original ggml_n_dims paramter type was not compatible with the type TensorStorage.

If you want, i will try to reproducate the original problem. If i remember correctly, the tensor_storage.n_dims was empty or corrupt.
Maybe it is no longer needed.

@fszontagh
Copy link
Contributor Author

I reproduced the n_dims problem. If i added a lora into the prompt (hair length slider lora) the following assesrtion happened:
image

@leejet
Copy link
Owner

leejet commented Feb 25, 2024

I reproduced the n_dims problem. If i added a lora into the prompt (hair length slider lora) the following assesrtion happened:

@fszontagh

I used the latest code from the master branch and did not encounter this issue. Can you try using the latest code from the master branch and see if the problem persists?

[INFO ] model.cpp:676  - load ..\models\hair_length_slider_v1.safetensors using safetensors format
[DEBUG] model.cpp:742  - init from '..\models\hair_length_slider_v1.safetensors'
[INFO ] lora.hpp:37   - loading LoRA from '..\models\hair_length_slider_v1.safetensors'
[DEBUG] model.cpp:1307 - loading tensors from ..\models\hair_length_slider_v1.safetensors
[DEBUG] ggml_extend.hpp:820  - lora params backend buffer size =   6.49 MB (10240 tensors)
[DEBUG] model.cpp:1307 - loading tensors from ..\models\hair_length_slider_v1.safetensors
[DEBUG] lora.hpp:67   - finished loaded lora
[DEBUG] ggml_extend.hpp:774  - lora compute buffer size: 100.02 MB
[INFO ] stable-diffusion.cpp:417  - lora 'hair_length_slider_v1' applied, taking 0.11s

@Cyberhan123
Copy link
Contributor

I have a guess about his question. In this file: https://github.com/leejet/stable-diffusion.cpp/blob/4a8190405ac32930678ce030dff6289ed680b6fc/.gitmodules#L3C44-L3C45
It points to the official ggml git, but now the merger of this PR: #159, it actually points to leejet/ggml, this is actually confusing for most people.

@fszontagh
Copy link
Contributor Author

fszontagh commented Feb 25, 2024

@leejet
It's new, because just compiled and tried to run the sd(.exe) without success. Here is the full command history
stable_diffusion_latest_full_cmd.txt

And here is again, but with a really full fresh start:
stable_diffusion_latest_full_cmd_2try.txt

In wsl it's working fine with lora too. Tested with a 256,6MB lora :)

@fszontagh
Copy link
Contributor Author

fszontagh commented Feb 25, 2024

Build with MSVC 2019 in vscode, and started the diffusion, but got:
Assertion failed: n_dims >= 1 && n_dims <= GGML_MAX_DIMS, file Z:\stable-diffusion.cpp_latest2\ggml\src\ggml.c, line 2678

Please see the full command
msvc2019.txt

without lora it's fine

@fszontagh
Copy link
Contributor Author

Same happening here with the auto builded release.
I downloaded the cuda version and tried to run, but it just stopped with a lora in the prompt.

@Cyberhan123

@fszontagh
Copy link
Contributor Author

fszontagh commented Feb 25, 2024

Another test, with the latest. I remade my changes with lora (n_dims). Then tried to reproducate an image with embedding:

[DEBUG] model.cpp:1307 - loading tensors from D:\SD_MODELS\embeddings\ng_deepnegative_v1_75t.pt
ggml_new_object: not enough space in the context's memory pool (needed 115632, available 32768)
Assertion failed: false, file Z:\stable-diffusion.cpp_latest2\ggml\src\ggml.c, line 2643

Another shot:

[INFO ] model.cpp:679 - load D:\SD_MODELS\embeddings\badhandv4.pt using checkpoint format
[DEBUG] model.cpp:1191 - init from 'D:\SD_MODELS\embeddings\badhandv4.pt'
[DEBUG] model.cpp:1307 - loading tensors from D:\SD_MODELS\embeddings\badhandv4.pt
[DEBUG] clip.hpp:923 - embedding 'badhandv4' applied, custom embeddings: 6
[DEBUG] clip.hpp:311 - split prompt ", text, title, logo, signature,username,bad anatomy, badhandv4,blurry, multiple ears, sharp teeth," to tokens [",", "text", ",", "title", ",", "logo", ",", "signature", ",", "username", ",", "bad", "anatomy", ",", ",", "blurry", ",", "multiple", "ears", ",", "sharp", "teeth", ",", ]
[DEBUG] clip.hpp:511 - clip_skip 2
ggml_gallocr_reserve_n: reallocating CUDA0 buffer from size 0.00 MiB to 72.62 MiB
[DEBUG] ggml_extend.hpp:774 - clip compute buffer size: 72.62 MB
[DEBUG] clip.hpp:511 - clip_skip 2
GGML_ASSERT: Z:\stable-diffusion.cpp_latest2\ggml\src\ggml-cuda.cu:8583: src0->type == GGML_TYPE_F32

@leejet
Copy link
Owner

leejet commented Feb 26, 2024

Another test, with the latest. I remade my changes with lora (n_dims). Then tried to reproducate an image with embedding

Currently, support for very large embeddings is not available. I will add it later.

@leejet
Copy link
Owner

leejet commented Feb 26, 2024

Build with MSVC 2019 in vscode, and started the diffusion, but got: Assertion failed: n_dims >= 1 && n_dims <= GGML_MAX_DIMS, file Z:\stable-diffusion.cpp_latest2\ggml\src\ggml.c, line 2678

Please see the full command msvc2019.txt

without lora it's fine

This issue is quite puzzling; I cannot replicate it in my local environment.

@fszontagh
Copy link
Contributor Author

fszontagh commented Feb 26, 2024

I build on a "virgin" PC (avx512), the same happening. Then i downloaded the prebuild binary to the same PC (avx512), that's working fine. But the downloaded cuda version is failed too on my machine.

Maybe the compiler causing this? In the CI that's an enterprise version, but i use community version.

@fszontagh
Copy link
Contributor Author

@leejet i givin up on this n_dims issue thing. But the 'progress callback' feature is a good feature. Do you accept the pr as-is?
On summary, the ggml_n_dims_t method causing nothing bad and maybe on later it will not be necessary after some new update, but currently it's working here at least.

@Cyberhan123
Copy link
Contributor

Same happening here with the auto builded release. I downloaded the cuda version and tried to run, but it just stopped with a lora in the prompt.

@Cyberhan123

I feel like your problem seems like you didn't pull the correct git submodel.

@fszontagh
Copy link
Contributor Author

@Cyberhan123 i tested with a fresh start too

@leejet
Copy link
Owner

leejet commented Feb 27, 2024

But the 'progress callback' feature is a good feature. Do you accept the pr as-is?

Certainly, I'm willing to merge this PR, or we can merge the progress callback first if you prefer.

@fszontagh
Copy link
Contributor Author

@leejet

Certainly, I'm willing to merge this PR, or we can merge the progress callback first if you prefer.

Okay. I pushed in it with removed ggml_n_dims_t method. After all, the 'progress callback' was the main point of this PR.

@leejet
Copy link
Owner

leejet commented Mar 2, 2024

Thank you for your contribution.

@leejet leejet merged commit 7be65fa into leejet:master Mar 2, 2024
rmatif pushed a commit to rmatif/stable-diffusion.cpp that referenced this pull request Apr 8, 2025
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.

4 participants