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

fix: fix the chat stuck in infinite loop #1755

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

zoe27
Copy link

@zoe27 zoe27 commented Jan 3, 2025

Relates to:

the chat stuck in infinite loop when using model_local #1213

Risks

Low

Background

What does this PR do?

This PR aim to fix the loop chat of the ai agent self when using the model_local

What kind of change is this?

modify the way of response generate in llama.ts

before this PR:

use the sequence.evaluate to generate the response

in this PR:

adjust to use the chatsession to generate the response

investigation step:

  • using LLAMALOCAL and HEURIST, issue for LLAMALOCAL but HEURIST works wells. which means maybe something wrong with LLAMALOCAL
  • using discord and direct client, HEURIST works well and LLAMALOCAL not well, which means no business with client but on the model
  • https://github.com/withcatai/node-llama-cpp reference to the git and use node-llama-cpp to load local model According to the guidance, it works well, which means the model self is OK
image - write a simple demo to test the model by using the `sequence.evaluate` , loop issue raised - reference to the node-llama-cpp code and adjust the code like in this PR, fix the issue

have not go to deeply to see what is the exactly different between sequence.evaluate and chatsession, maybe can fix loop bug first, and for now, have not found any risk.

Documentation changes needed?

Testing

Where should a reviewer start?

Detailed testing steps

Before
it loop the response and sometimes, the response is hard to understood
image
After
seems in a normal way to chat
image

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hi @zoe27! Welcome to the ai16z community. Thanks for submitting your first pull request; your efforts are helping us accelerate towards AGI. We'll review it shortly. You are now a ai16z contributor!

@marcellodesales
Copy link
Contributor

@zoe27 The PR is failing with the check against the main branch... Change it to develop to clean the PR checks!

https://github.com/elizaOS/eliza/actions/runs/12597379513/job/35110258487?pr=1755#step:2:8

@zoe27 zoe27 changed the base branch from main to develop January 3, 2025 14:23
@odilitime odilitime changed the title fix the chat stuck in infinite loop fix: fix the chat stuck in infinite loop Jan 3, 2025
const tokens = this.model!.tokenize(context);

// tokenize the words to punish
const wordsToPunishTokens = wordsToPunish
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems like functionality we'd want to keep, is this critical to remove to fix the infinite loop functionality?

Copy link
Author

Choose a reason for hiding this comment

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

No, updated it and keep the function

const responseTokens: Token[] = [];

for await (const token of this.sequence.evaluate(tokens, {
temperature: Number(temperature),
Copy link
Collaborator

Choose a reason for hiding this comment

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

definitely need it to pay attention to temperature

Copy link
Author

Choose a reason for hiding this comment

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

updated it, and keep the temperature in the generating function

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