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

src: remove duplicate function llama_should_add_bos_token #8778

Merged
merged 1 commit into from
Aug 15, 2024

Conversation

kylo5aby
Copy link
Contributor

@kylo5aby kylo5aby commented Jul 30, 2024

  • I have read the contributing guidelines
  • Self-reported review complexity:
    • Low
    • [] Medium
    • High
      Since this PR has update tokenizer_add_bos and tokenizer_add_eos the behavior of llama_should_add_bos_token has also been changed, the PR remove this function and update the return type of related functions.

@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 Aug 1, 2024
@kylo5aby
Copy link
Contributor Author

kylo5aby commented Aug 5, 2024

can you please take a look @jaime-m-p @ggerganov

@mofosyne mofosyne added the refactoring Refactoring label Aug 6, 2024
@ggerganov ggerganov merged commit 4af8420 into ggerganov:master Aug 15, 2024
52 checks passed
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 15, 2024
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
examples refactoring Refactoring Review Complexity : Low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants