I couldn't have much time to look into details, but can you try:

1. Not to call llama_control_vector_init in POST /control-vectors? Because this function allocate devices buffer (i.e. GPU memory), it should be call only once (when the program starts)
2. llama_control_vector_apply does not take into account scale or multiple vectors. Maybe for this first version, can we just replace the current vector, then later on, we will see how to merge 2 vectors and control their scales at runtime
3. llama_control_vector_load should be called when the program starts, because it reads vectors from disk and save to RAM. It should not be called inside POST /control-vectors

@ngxson

I couldn't have much time to look into details, but can you try:

1. **Not** to call `llama_control_vector_init` in `POST /control-vectors`? Because this function allocate devices buffer (i.e. GPU memory), it should be call only once (when the program starts)

Yeah, that's what I initially tried. Originally the llama_control_vector_init was guarded by asserts, checking that its buffers and tensors are empty to make sure it doesn't get initialized twice. But after loading a control vector that way (without calling init again) the output does not reflect the behaviour of the control vector. Instead, when loading a vector that way it seems as if no control vector is applied anymore, or when doing it again and again, trying different vector combinations, it just becomes increasingly incoherent, with no recognizable relation to the normal control vector behaviour.

So it seems something has to be cleaned up / reinitialized when wanting to apply a different control vector later. But I don't know what exactly. I just don't understand all that lower-level stuff that's going on there. So I simply tried this brutish method in my last commit to see what happens. Removing the assert guards and forcefully calling llama_control_vector_init a second time does seem to show the correct behaviour of the newly applied control vector, but segfaults very soon.

Loading control vectors with the guards in place (no repeated call of llama_control_vector_init) also led to segfaults, but much later down the line. However, as mentioned, with no recognizable relation to the normal control vector behaviour.

2. `llama_control_vector_apply` does not take into account scale or multiple vectors. Maybe for this first version, can we just replace the current vector, then later on, we will see how to merge 2 vectors and control their scales at runtime

llama_control_vector_apply applies the combined control vector that has been assembled by llama_control_vector_load from an arbitrary number of vectors with coefficients. This functionality was not added by me, but in the earlier PR from @vgel. And it works just fine.

3. `llama_control_vector_load` should be called when the program starts, because it reads vectors from disk and save to RAM. It should not be called inside `POST /control-vectors`

But swapping out control vectors live and being able to change them akin to samplers was my main motivation.
It seems to work quite fast. Only problem is that it's broken. :D

@phymbert

Also, please implement the control_vectors.feature in the server test framework within the PR. Then, we need to assess the impact of control vectors on the server performances.

Sorry, I have no idea how that works, and when I try to install the python requirements I get errors thrown at me:

...
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1381, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1354, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1304, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 488, in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 1381, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1354, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1325, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 929, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 994, in exec_module
  File "<frozen importlib._bootstrap>", line 488, in _call_with_frames_removed
  File "/tmp/pip-build-env-49api5t8/overlay/lib/python3.12/site-packages/setuptools/__init__.py", line 10, in <module>
    import distutils.core
ModuleNotFoundError: No module named 'distutils'

trollkotze@schrotthaufen:~/llama/llama.cpp/examples/server/tests$ pip install distutils
ERROR: Could not find a version that satisfies the requirement distutils (from versions: none)
ERROR: No matching distribution found for distutils

Maybe you could take over from here? I'm afraid of snakes.

@phymbert I just noticed one peculiar behaviour: Sometimes the control vector has no effect. I think that happens when a request comes in before the initialization of the model, context + vector is completed. (The server does not respond in that case, but it affects its behaviour later on when everything is initialized and requests are served: The control vector specified through command line argument just has no effect then.)
Maybe this is a more general issue with some race condition that might also affect other behaviour, and then it might be out of scope of this PR. Not sure. But in any case it is outside the scope of my competence.

Maybe you could take over from here? I'm afraid of snakes.

No sorry, IMHO adding this test is straight forward: add a new step and new server params and copy a completion scenario.

For python, just create a python venv.

Reopening this PR again.
This PR without the initially attempted hot-reloading functionality is trivial. And I don't really care enough about it to follow up with additional requirements. So I closed it. I don't know about python venv stuff and how to use that test framework. And if someone says that it's straightforwad then he should just do it.

But some anon suggested I should simply provide a good argument why this should be merged as is.
Here it is: There are plenty of other command-line parameters like this, which simply use the same logic as in main.cpp with all the logic that is already in place for that, handled in llama_init_from_gpt_params, like e.g. for Lora adapters and whatnot. There are no test cases for most of these.

I see no fundamental difference here for these control vector arguments and why they should require special testing in server.cpp when the functionality is the same in main.cpp.

So take it or leave it. I think it makes sense to just merge this.

And about my original intention of adding hot-reloading for control vectors in server.cpp:
Some anon said the reason why my attempt did not work is a memory leak in llama_control_vector_load, and I think someone is already looking into that and maybe following up with a fix. And then maybe that will make it possible to actually add hot-reloading functionality for control vectors.
About the argument of not allowing access to arbitrary file paths for control vector loading on the server, one could add command line parameters to specify a set of named control vector options to choose from. That would be pretty cool.
But that's for another future PR then that hopefully someone will implement when the issues with llama_control_vector_load are fixed.

Reopening this PR again. This PR without the initially attempted hot-reloading functionality is trivial. And I don't really care enough about it to follow up with additional requirements. So I closed it. I don't know about python venv stuff and how to use that test framework. And if someone says that it's straightforwad then he should just do it.

But some anon suggested I should simply provide a good argument why this should be merged as is. Here it is: There are plenty of other command-line parameters like this, which simply use the same logic as in main.cpp with all the logic that is already in place for that, handled in llama_init_from_gpt_params, like e.g. for Lora adapters and whatnot. There are no test cases for most of these.

I see no fundamental difference here for these control vector arguments and why they should require special testing in server.cpp when the functionality is the same in main.cpp.

So take it or leave it. I think it makes sense to just merge this.

I am sorry you feel it this way, adding this feature is great, but we aim to provide a production ready server, not a playground. A feature is nothing without proper test implementation in order to maintain it. You cannot ask people to do tests for you. As you do not "care enough", I am adding the demo label in case other people want to bring to master properly.

Yes, there is no rush to add this functionality to server immediately. If you feel strongly about having proper tests before merging, we can do that - would be a good opportunity for new people to make a contribution

If you feel strongly about having proper tests before merging, we can do that - would be a good opportunity for new people to make a contribution

Thanks for your understanding. Let's discuss this again if someone really needs this to master one day. Meanwhile, the linked issue is opened for contributions.

Okay. Here is all I've got:

Anon shared a patch here that fixes the memory allocation/freeing bugs in llama_control_vector_load. So based on that, I do care again, because I could include the hot-swap functionality again on top of that patch, and it works now without segfault.

To address the security concern about accessing arbitrary control vector files through that hot-swapping endpoint, it is now necessary to provide predefined options for control vectors that can be loaded:

1. option for specific gguf file:
   --control-vector-option name /path/to/vector.gguf
2. option for specific directory:
   --control-vector-option name path/to/dir
--control-vector-option name path/to/dir/

The server will decide based on the specified path string whether it is interpreted as a vector file or a directory. A slash will be automatically added to the name of a directory option.
There is a new endpoint to query the available vector options: GET /control-vector-options will return a json array of strings.
So e.g. --control-vector-option vectors path/to/dir --control-vector-option victor path/to/some/vector.gguf will result in the listed options: ["vectors/", "victor"]. So then the user would be allowed to load vectors/anything/goes/here (except path traversal) or victor.
Path traversal is checked and guarded against.

Hot-reloading works the same as before, but without segfaults.
And now you specify the given name instead of the file path.
For given directory options you can load anything like givendirectoryname/path/to/file or givendirectoryname/path/to/file.gguf (which are equivalent here).
Path traversal is guarded against to the extent I am aware of.

That is all I'm willing and able to do and what I will be using myself. If someone likes to fix it up with all the necessary compliance bullshit to merge it into master that would of course be very nice. Don't forget to give credit to anon@example.com for his contributions of commits 181879f, 9914014, and d0304f7.

I'm curious, how does this deal with parallel execution?

I assume that the applied control vector affects all "conversations" in a multi-user context?

I'm curious, how does this deal with parallel execution?

I assume that the applied control vector affects all "conversations" in a multi-user context?

Yes, it does. I guess it would be possible to decouple user settings for control vectors by adding control vector options to the completion endpoint instead of just this "global" control vector endpoint. It looks a bit complicated because of the async stuff with the request queue going on in handle_completions. The appropriate control vector would need to be set when the request is actually executed.

I assume that the applied control vector affects all "conversations" in a multi-user context?

Control vectors are context-independent, because it is applied to the embedding vector between layers, not the self-attention mechanism.

@trollkotze Thanks for picking this up (and catching the memory bugs in my original implementation 😅) I've been on vacation so haven't been checking progress, but I'm experienced w/ Python, would be happy to coordinate to get this over the line for merging! (I'm @vgel on Discord if you use it.)

@vgel Thanks! That would be great if you could help out here and do the python stuff or whatever else there is to do.
Credit for fixing the memory bug goes to Anon. I really still don't understand too much about these details.
I am quite busy at the moment and for the next weeks and am really not well-versed in the llama.cpp code, and even less in python, conda environments and the purpose of these testing frameworks, so I would be most happy if someone could just pick it up from here, either by adding to my PR or by just copying and reworking it, and changing whatever is necessary to get it merged.
I just added you as collaborator in my fork. Feel free to do whatever you want there. If anyone else is interested, let me know. But of course it migh be more convenient to just make your own fork and rework it there, and then I don't have to care about rights management in GitHub.
I also added you in Discord (Hotzenplotz), but I don't really use it. Usually GitHub notifications will reach me earlier that that.
There is also another newer branch "control-vector-server-unclean" in my llama.cpp forked repo where I did some other stuff, I forgot exactly what atm. I think mainly just loading all the control vector files in a directory and showing them listed individually under options when specifying a directory instead of a file for a control vector parameter, so that the user can see the files listed in the options instead of having to know what files under that dir exist. But that is only for Linux and I don't have Windows. Alternatively, of course, one could simply add a long list of all the individual control vector files as options that the user can then see, as is already possible with the code in this cleaner branch here.

@phymbert I also invited you to my fork as collaborator in case you feel motivated and want to fiddle with it there.
But of course, as I said, whoever is interested, may feel free to just copy the code, fork it again, make a new PR. Whatever might be the more convenient way to do it.