Skip to content

[Suggestion] Reduce API surface of ggml.h #1186

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

Closed
howard0su opened this issue Apr 26, 2023 · 3 comments
Closed

[Suggestion] Reduce API surface of ggml.h #1186

howard0su opened this issue Apr 26, 2023 · 3 comments
Labels

Comments

@howard0su
Copy link
Collaborator

The latest ggml.h expose all the details of ggml_tensor and the related OP names. This may causes compatibility problems later as the consumer may direct access the fields of tensor. In the world of binary compatibility, this introduce maintenance headache.

Proposed Changes:

  1. hide ggml_tensor details, add couple of helper functions like ggm_tensor_size, _nb, _ne etc to expose necessary information.
  2. Create ggml-internal.h and move internal interface to that including tensor struct, op definitions etc.

If we agree, I can propose a change.

@howard0su
Copy link
Collaborator Author

@ggerganov love to hear you option before we move forward.

@ggerganov
Copy link
Member

Such change can happen at a much later stage of the development.
Right now, it's more important to access the structs in a comfortable and convenient way - no need to worry about binary compatibility.

Later, let's say for V1, such change could be made.
But now it will just make our lives harder having to write getters and setters all the time.

@github-actions github-actions bot added the stale label Mar 25, 2024
Copy link
Contributor

github-actions bot commented Apr 9, 2024

This issue was closed because it has been inactive for 14 days since being marked as stale.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants