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

backend: support non-ASCII characters in path to llmodel libs on Windows #2388

Merged
merged 11 commits into from
May 31, 2024

Conversation

cebtenzzre
Copy link
Member

There were two problems preventing GPT4All from locating the implementation libraries on Windows if the path contained non-ASCII characters:

  • We were using LoadLibraryExA (which normally only supports ASCII) and not LoadLibraryExW
  • std::filesystem doesn't understand UTF-8 on Windows and must be used with wide strings instead

While I was touching this code I refactored Dlhandle into a proper .cpp/.h split, which greatly improves readability IMO.

Related to #2111 (that issue was about the model path, not the implementation lib path)

Testing

I created a venv called zz😊 (for tab completion's sake) using pure Python (since using Unicode at the Windows command line is not trivial):

$ python
>>> import sys
>>> sys.argv[1:] = ['zz\U0001F60A']
>>> import venv.__main__

Then I activated it (the glob allows this to work even when the console doesn't fully support Unicode):

$ . .\zz??\Scripts\Activate.ps1

Then install the Python binding in that venv. With the current release, this fails:

$ python
>>> from gpt4all import GPT4All
>>> x = GPT4All('orca-mini-3b-gguf2-q4_0.gguf') 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\msys64\home\jared\gpt4all\zz😊\Lib\site-packages\gpt4all\gpt4all.py", line 206, in __init__
    self.model = LLModel(self.config["path"], n_ctx, ngl)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\msys64\home\jared\gpt4all\zz😊\Lib\site-packages\gpt4all\_pyllmodel.py", line 218, in __init__
    raise RuntimeError(f"Unable to instantiate model: {'null' if s is None else s.decode()}")
RuntimeError: Unable to instantiate model: Could not find any implementations for build variant: default

But with this PR (don't install it editable/-e or the resolved path won't be in the venv), it works:

$ python
>>> from gpt4all import GPT4All
>>> x = GPT4All('orca-mini-3b-gguf2-q4_0.gguf')
>>>

I didn't test with the UI since the installer doesn't let you select a path containing non-ASCII characters, but if the user moves GPT4All to a non-ASCII path after-the-fact, this should at least give it a chance of working.

Signed-off-by: Jared Van Bortel <jared@nomic.ai>
Signed-off-by: Jared Van Bortel <jared@nomic.ai>
Using the native path representation allows us to manipulate paths and
call LoadLibraryEx without mangling non-ASCII characters.

Signed-off-by: Jared Van Bortel <jared@nomic.ai>
@cebtenzzre cebtenzzre requested a review from manyoso May 29, 2024 17:45
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>
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>
Signed-off-by: Jared Van Bortel <jared@nomic.ai>
Signed-off-by: Jared Van Bortel <jared@nomic.ai>
@manyoso manyoso merged commit 4e89a9c into main May 31, 2024
6 of 19 checks passed
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