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

wrap llama_batch_get_one #579

Merged
merged 3 commits into from
Nov 27, 2024
Merged

wrap llama_batch_get_one #579

merged 3 commits into from
Nov 27, 2024

Conversation

tinglou
Copy link
Contributor

@tinglou tinglou commented Nov 21, 2024

wrap llama_cpp_sys_2::llama_batch_get_one and safer drop

///
pub fn get_one(tokens: &[LlamaToken], pos_0: llama_pos, seq_id: llama_seq_id) -> Self {
unsafe {
let ptr = tokens.as_ptr() as *mut i32;
Copy link
Contributor

@MarcusDunn MarcusDunn Nov 21, 2024

Choose a reason for hiding this comment

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

Why does this need to be mut? If it doesn't, can you make a PR to llama-cpp instead of casting? If it does need to be mut, that should be reflected in the signature.

Also what are the implications of an empty slice here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#[repr(C)]
#[derive(Debug, Copy, Clone, PartialEq)]
pub struct llama_batch {
    pub n_tokens: i32,
    pub token: *mut llama_token,
    pub embd: *mut f32,
    pub pos: *mut llama_pos,
    pub n_seq_id: *mut i32,
    pub seq_id: *mut *mut llama_seq_id,
    pub logits: *mut i8,
    pub all_pos_0: llama_pos,
    pub all_pos_1: llama_pos,
    pub all_seq_id: llama_seq_id,
}

The orignal field token is mut.

Yes, the slice should not be empty.

@MarcusDunn MarcusDunn merged commit d3eade6 into utilityai:main Nov 27, 2024
2 of 5 checks passed
@MarcusDunn
Copy link
Contributor

included in latest, thanks for the PR

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.

2 participants