-
-
Notifications
You must be signed in to change notification settings - Fork 31
Make the constrained beam finish the current word mid-word #520
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -150,6 +150,24 @@ final class ConstrainedBeamSearchTests: XCTestCase { | |
| XCTAssertFalse(recorder.paths.contains([0, 1]), "search must stop at the sentence and not step past it") | ||
| } | ||
|
|
||
| func test_search_midWord_firstTokenMustContinueTheWord() { | ||
| // token 0 breaks the word (leading punctuation) but has the higher logit; token 1 continues it. | ||
| // Mid-word, only a word-continuing token may start the completion. | ||
| let profile = makeProfile(byteStrings: [", and", "ing"]) | ||
| let rows: [[Int]: [Float]] = [[]: row([0: 9, 1: 1], vocabSize: 2)] | ||
| let normal = ConstrainedBeamSearch.search( | ||
| nextLogits: provider(vocabSize: 2, rows: rows), profile: profile, | ||
| configuration: BeamSearchConfiguration(beamWidth: 1, maxTokens: 1, topK: 5), | ||
| isSingleLine: false, isMidWord: false) | ||
| let midWord = ConstrainedBeamSearch.search( | ||
| nextLogits: provider(vocabSize: 2, rows: rows), profile: profile, | ||
| configuration: BeamSearchConfiguration(beamWidth: 1, maxTokens: 1, topK: 5), | ||
| isSingleLine: false, isMidWord: true) | ||
|
|
||
| XCTAssertEqual(normal.first?.tokenIDs, [0], "without mid-word, the highest-logit token wins") | ||
| XCTAssertEqual(midWord.first?.tokenIDs, [1], "mid-word, the word-breaking token is filtered out") | ||
| } | ||
|
|
||
| func test_search_respectsMaxTokenBudget() { | ||
| // No EOG / sentence end: every token keeps generating, so the budget bounds the length. | ||
| let profile = makeProfile(byteStrings: ["a", "b"]) | ||
|
Comment on lines
150
to
173
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The new test covers the normal filtered case, but there's no assertion for when Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isControlis never consulted — onlybytes.isEmptyis checked. A non-control token whose bytes happen to be empty would also returnfalse, and a control token with non-empty bytes starting with a letter would returntrue(thoughrankedAdmissibleTokenswould already have excluded it). The parenthetical is misleading; tightening the wording removes the ambiguity.