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

[experimental] ggml C++ bindings #581

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

[experimental] ggml C++ bindings #581

wants to merge 3 commits into from

Conversation

slaren
Copy link
Collaborator

@slaren slaren commented Oct 15, 2023

Adds an example with C++ bindings, and the gpt-2 example adapted to use these bindings.

At this point this is mostly an experiment to see how ggml C++ bindings could look like. I am not sure if this is something that we may want at all.

The advantage over the C API would be automatic resource freeing, and maybe a nicer syntax in some cases.

Copy link
Contributor

@Green-Sky Green-Sky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make c++ headers be .hpp ?

@slaren
Copy link
Collaborator Author

slaren commented Oct 15, 2023

I think using .h or .hpp is mostly a matter a matter of style. Generally I prefer to use .h for C++ too, but I don't really care either way.

Copy link
Owner

@ggerganov ggerganov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The graph building code looks very clean.
The context stack should probably be thread_local so that we can build graphs in parallel threads.

I am also not sure if we need the C++ bindings. Some things indeed get much easier to implement in C++. Maybe if we limit the bindings scope to some very basic things like auto deallocation of resources and scoped-contexts it wouldn't be a problem to maintain them.

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.

3 participants