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

feat: optimize the native library loading. #688

Merged
merged 11 commits into from
May 22, 2024

Conversation

AsakusaRinne
Copy link
Collaborator

@AsakusaRinne AsakusaRinne commented Apr 23, 2024

Implement the proposal in #670

Public API changes

Add NativeLibraryConfig.WithAutoDownload

By default it's disabled and the user could enable it with some settings.

Add NativeLibraryConfig.WithSelectingPolicy

To allow users to customize the auto-selection process. Users could return a list of paths from INativeLibrarySelectingPolicy.Select , ordered by the priority.

Make NativeLibraryConfig.Description public

I made this change to simplify some implementations. It seems not to be risky to expose this class.

Add NativeLibraryConfig.All, NativeLibraryConfig.LLama, NativeLibraryConfig.LLavaShared and make NativeLibraryConfig.Instance obsolete.

There has been two native libraries now, so the name Instance seems no longer reasonable. With this modification, users could specify configurations for one of them. Meanwhile, using NativeLibraryConfig.All will apply same configurations to both of them.

Add NativeLibraryConfig.VersionMap

Please tell me if there's a better design. I added it to provide a mapping from version tag (such as v0.11.2) to the llama.cpp commit hash, which is also the revision of huggingface remote branch.

Add NativeLibraryConfig.DryRun

This API executes the whole loading process except assigning the loaded pointer to NativeApi. I added it for the following two reasons.

  1. It will allow user to check the configuration at the runtime. If it returns false, the user could have chances to revise the configuration before loading the model.
  2. The downloading is actually a asynchronized process. Though I synchronized it by calling Task.Result, I'm still not sure whether there's potential problem using async method in a static constructor. So, currently, users are supposed to call DryRun when auto-download is enabled.

Other changes

  • Add the package HuggingfaceHub as a dependency.
  • Adjust the file structures to make it more clear.

TODO

  • Windows test
  • Linux test
  • MAC test
  • .NET standard2.0 test

How to test it

Comment all the content in LLama/LLamaSharp.Runtime.targets and run the examples. It's better to enable the full logs (though in a bit mess now).

@martindevans
Copy link
Member

Can this be split into two parts (in two PRs):

  1. Most of the changes you've made here to enable downloading. This would include splitting the config, the dry run etc.
  2. The actual downloader implementation.

With that split we've exposed all of the parts required for custom downloaders to be implemented outside of the project, which then means people can use this auto download infrastructure but without having to trust our binaries at all.

If that separation is made we can also have the debate about if LLamaSharp should include a default implementation, I'm strongly of the opinion it shouldn't. We can provide a downloader as a separate package if we want (e.g. LLamaSharp.backend.AutoDownload backend). This would also mean we avoid adding another external dependency to LLamaSharp.

@AsakusaRinne
Copy link
Collaborator Author

I'll modify this PR to leave only the parts not related with downloading and add the other as a separate PR.

@AsakusaRinne AsakusaRinne changed the title feat: support auto-download for native libraries. feat: optimize the native library loading. Apr 24, 2024
@AsakusaRinne
Copy link
Collaborator Author

@martindevans I've removed all things related with auto-download.

@martindevans
Copy link
Member

I haven't gone through this fully in depth, but there's a few minor nitpicks I noticed above.

Overall this looks like a good direction. I like the idea to split the various type of native libs up into separate files altogether, that'll get some of the complexity of the system udner control!

@AsakusaRinne
Copy link
Collaborator Author

Perhaps I'm going to merge this PR in several days. #692 and #738 will be both blocked by this PR. Test in windows, linux, MAC and .NET standard2.0 have all been passed. I'll fix the possible other Bugs after this PR.

@SignalRT SignalRT self-requested a review May 20, 2024 15:49
@AsakusaRinne AsakusaRinne merged commit ab4c7a9 into SciSharp:master May 22, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request native lib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants