Skip to content

llama: fix some return values to match hparams types #8461

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
wants to merge 1 commit into from

Conversation

iboB
Copy link
Contributor

@iboB iboB commented Jul 12, 2024

Nitpick mostly, but there seems to be no reason to cast the values from llama_hparams to from uint to int.

@iboB
Copy link
Contributor Author

iboB commented Jul 12, 2024

Just found out that this also affects examples. Should I fix them or should the PR be abandoned?

@ggerganov ?

@compilade
Copy link
Collaborator

This seems related to #4574 and #4577

(there are also relevant comments about signed vs unsigned integer types like #4540 (review))

@mofosyne mofosyne added the Review Complexity : Low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix label Jul 13, 2024
@ggerganov
Copy link
Member

Let's close this - I'll try to change most of the uint32_t hparams to int32_t in a follow-up refactoring pass

@ggerganov ggerganov closed this Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review Complexity : Low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants