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

Add repetition test #1339

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add repetition test #1339

wants to merge 1 commit into from

Conversation

jiafatom
Copy link

No description provided.

Comment on lines +90 to +91
ngram_len_threshold = 10
repeat_threshold = 5
Copy link
Member

Choose a reason for hiding this comment

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

Are these values standard or should they be adjustable? If possible, a comment explaining why these values were chosen.

Copy link
Author

Choose a reason for hiding this comment

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

This is for internal test, so I just treat these numbers to start with.

@@ -154,5 +173,6 @@ def main(args):
parser.add_argument('-c', '--chat_template', type=str, default='', help='Chat template to use for the prompt. User input will be injected into {input}')
parser.add_argument('-s', '--system_prompt', type=str, default='You are a helpful assistant.', help='System prompt to use for the prompt.')
parser.add_argument('-r', '--rewind', action='store_true', default=False, help='Rewind to the system prompt after each generation. Defaults to false')
parser.add_argument('-ret', '--repetition_test', action='store_true', default=False, help='Perform repetition test. Defaults to false')
Copy link
Member

Choose a reason for hiding this comment

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

Is it a test or more like 'repetition_metrics'? At first I thought this PR was adding a unit test.

Copy link
Author

Choose a reason for hiding this comment

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

This test here is for debugging purposes. We want to test whether the repetition happens in output, for the ongoing investigation of some of models generated repetitions after some point.

Copy link
Member

Choose a reason for hiding this comment

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

Should this be in our public examples, if it's for testing perhaps this should be in test\python?

Copy link
Author

Choose a reason for hiding this comment

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

do we expect public users to use model-chat.py? If so, I am fine to use test/python, which file may be good to add these utility?

Copy link
Contributor

Choose a reason for hiding this comment

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

Many users are using the posted examples to run with ONNX Runtime GenAI and these examples are linked in many public tutorials as well. We can add this debugging test in the end-to-end unit tests here instead.

@@ -87,6 +87,8 @@ def main(args):
system_tokens = tokenizer.encode(system_prompt)
generator.append_tokens(system_tokens)
system_prompt_length = len(system_tokens)
ngram_len_threshold = 10
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ngram_len_threshold = 10
ngram_threshold = 10

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