Skip to content
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

chat: fix blank device in UI after model switch and improve Mixpanel reporting #2409

Merged
merged 9 commits into from
Jun 26, 2024

Conversation

cebtenzzre
Copy link
Member

Functional Changes

  • When you switch chats in the UI and they both use the same model, the device and fallback reason reported in the lower-right corner of the chat no longer become blank.
  • In Mixpanel, actualDevice was blank about 7% of the time because of the above issue. This is now fixed.
  • In Mixpanel, v2.8.0 reported devices as e.g. actualDevice="GTX 970 (CUDA)" and actualDevice="Tesla P40 (Vulkan)". Now they are reported as actualDevice="GTX 970", device_backend="CUDA" and actualDevice="Tesla P40", device_backend="Vulkan".
  • default_device on Mixpanel still shows plain device names in v2.8.0. default_device_backend is added to supplement this (should always be "Vulkan" in official releases).

Other Changes

  • Remove LLModel::hasGPUDevice and llmodel_has_gpu_device. In current GPT4All, it is always true if the last call to initializeGPUDevice returned true. There are no users of this function. (cc @jacoobes @iimez - the ts bindings still reference this, likely incorrectly)
  • Replace the llama.cpp hack that sets n_gpu_layers to zero when Kompute falls back to CPU with a more reasonable API that introduces llama_model_using_gpu.

This function has never been used and is equivalent to whether the last
call to initializeGPUDevice() returned true.

Signed-off-by: Jared Van Bortel <jared@nomic.ai>
Signed-off-by: Jared Van Bortel <jared@nomic.ai>
This fixes the issue where reusing a model for a new chat would cause
the device name and fallback reason to be lost - both in the UI, and on
Mixpanel.

Also, separate the device name from the backend (now at the
"deviceBackend" prop) on Mixpanel.

Signed-off-by: Jared Van Bortel <jared@nomic.ai>
Signed-off-by: Jared Van Bortel <jared@nomic.ai>
Signed-off-by: Jared Van Bortel <jared@nomic.ai>
@cebtenzzre cebtenzzre requested a review from manyoso June 4, 2024 20:00
@cebtenzzre
Copy link
Member Author

The merge conflicts will be resolved after #2408 is merged.

iimez added a commit to iimez/gpt4all that referenced this pull request Jun 4, 2024
Signed-off-by: limez <limez@protonmail.com>
Signed-off-by: Jared Van Bortel <jared@nomic.ai>
QVariant() is `undefined` in QML, while QString() is `null`.

Signed-off-by: Jared Van Bortel <jared@nomic.ai>
When we do CPU fallback and request CUDA with ngl=0, it still uses the
GPU for matrix multipliciation. It shouldn't clear the GPU device on
failure, as it is still relevant when we fall back. We also shouldn't
allow loadModel with an unset device.

This fixes a case where gpuDeviceName() would return "", because
usingGPUDevice() reported true while we had no known device name.

Signed-off-by: Jared Van Bortel <jared@nomic.ai>
@cebtenzzre cebtenzzre merged commit 01870b4 into main Jun 26, 2024
6 of 18 checks passed
@cebtenzzre cebtenzzre deleted the fix-cur-gpu-device branch June 26, 2024 19:26
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.

None yet

2 participants