llama.cpp : split llama_context_params into model and context params

[slaren]

Currently, llama_load_model_from_file and llama_new_context_with_model receive the same parameters, but it is not specified which parameters are relevant for each object. This change splits the parameters into llama_model_params and llama_context_params so that it is clear what parameters can be modified per-context.

Additionally:

• Uses the model n_ctx_train as the default context size when llama_context_params::n_ctx == 0 or -c 0
• Moves n_threads to llama_context_params, adds n_threads_batch and the corresponding command line argument -tb, --threads-batch for batches/prompt processing
• Removes the low_vram parameter and the corresponding -lv, --low-vram command line argument

[slaren]

Despite what the prints say, it looks like the KV cache is offloaded regardless of the value of low_vram. Am I missing something here, or did this functionality get lost somewhere along the way? Tagging @JohannesGaessler.

llama.cpp/llama.cpp

Lines 2323 to 2338 in bc9d3e3

	if (n_gpu_layers > (int) hparams.n_layer + 1) {
	if (low_vram) {
	LLAMA_LOG_INFO("%s: cannot offload v cache to GPU due to low VRAM option\n", __func__);
	} else {
	LLAMA_LOG_INFO("%s: offloading v cache to GPU\n", __func__);
	vram_kv_cache += hparams.kv_size() / 2;
	}
	}
	if (n_gpu_layers > (int) hparams.n_layer + 2) {
	if (low_vram) {
	LLAMA_LOG_WARN("%s: cannot offload k cache to GPU due to low VRAM option\n", __func__);
	} else {
	LLAMA_LOG_INFO("%s: offloading k cache to GPU\n", __func__);
	vram_kv_cache += hparams.kv_size() / 2;
	}
	}

llama.cpp/llama.cpp

Lines 1229 to 1236 in bc9d3e3

	#ifdef GGML_USE_CUBLAS
	if (n_gpu_layers > n_layer + 1) {
	ggml_cuda_assign_buffers_no_scratch(cache.v);
	}
	if (n_gpu_layers > n_layer + 2) {
	ggml_cuda_assign_buffers_no_scratch(cache.k);
	}
	#endif // GGML_USE_CUBLAS

[JohannesGaessler]

Both the --low-vram option and KV cache offloading were added in #1827 . It seems that the program logic and the prints were inconsistent from the start. On master the results are still correct when combining --low-vram with KV cache offloading. However, the performance degrades relative to offloading only the repeating and non-repeating layers due to a much higher number of CPU<->GPU copies. I don't remember for certain what I did several months ago but I think the reason the prints say that the combination of --low-vram and KV cache offloading is not allowed is because supporting the combination would not make sense in terms of performance and I wanted to reduce the work needed for implementation/maintenance.

I'm not 100% sure how to proceed. The results happen to be correct, but as I said: supporting the combination does not make sense and I would not consider it to be in-scope. So I personally would still prefer to disallow it and to adjust the program logic to match the prints.

[slaren]

The reason this is relevant to this PR is that currently I have duplicated low_vram in llama_model_params and llama_context_params, since this parameter is supposed to be used both when loading the model and when creating the KV cache. I don't think this is good, and I would prefer to remove it from llama_context_params. It would still be possible to do this and to match the logic of the prints by storing the value of low_vram in llama_model, but I am not convinced that it is worth to do that, since the same behavior can be achieved simply by using a smaller value of n_gpu_layers.

There is also the issue that the only documentation of low_vram (in the command line help) is that it disables the VRAM scratch buffer, however I don't think that disabling the scratch buffer at the moment makes a lot of sense, since it can be reduced to almost nothing by using a low batch size, so the cost that this has on the performance is probably disproportionate compared to the amount of VRAM that it saves. I am not even sure that disabling the VRAM scracth buffer does anything now, since this memory will still be necessary to store the results of the operations, but it will be allocated from the CUDA memory pool instead.

[JohannesGaessler]

Alright. I do not feel strongly about this and my line of reasoning is revolving around what is easier to implement and maintain anyways (assuming we want to keep --low-vram that would be to disallow the combination with KV cache offloading). Intuitively I would agree that thanks to your improvements regarding VRAM allocation (and performance optimizations in general) the use of --low-vram probably no longer makes sense and I would be fine with just removing the option. I cannot 100% guarantee that there isn't still some use case where --low-vram helps but if there are any they should be very rare.

[slaren]

I ran some tests with low_vram, and besides some unintended consequences of disabling the scratch buffer while still offloading the KV (some mat muls during attention wouldn't be offloaded anymore), I couldn't observe any meaningful differences in VRAM usage. After removing the effect of low_vram on the VRAM scratch buffer, the only remaining effect was preventing the offloading of the output norm weight, which is tiny anyway, just 16 KB on 7B. So I went ahead and removed the low_vram option entirely.

[ggerganov]

We can merge this together with #3228 to make the API change in one go

[netrunnereve]

Thanks for completing #2534! It looks and runs fine on my end.

Note there are a few things from my PR that you may want to add here, such as README updates and including the thread count in the system info line.

[slaren]

While we are changing the API, should we also remove all the _with_model functions, and make the regular functions take a llama_model instead? I have already added a llama_get_model API to obtain the model of a context, so that users that only have a llama_context for whatever reason can still use these functions.

[ggerganov]

While we are changing the API, should we also remove all the _from_model functions

Do you mean _with_model?
Yes, we should do that. Lets resolve conflicts and merge

[slaren]

Should we change this to something like std::thread::hardware_concurrency(), or get_num_physical_cores() from common.cpp? It shouldn't affect most of our examples, but a better default could help downstream users.

[ggerganov]

The best option would be to move get_num_physical_cores() to ggml and use it.
But can be done in separate PR

[slaren]

If the API changes look good I think this can be merged.