Skip to content

Fix Batching Issues#1045

Merged
martindevans merged 2 commits intoSciSharp:masterfrom
martindevans:investigate_batching_issues
Jan 17, 2025
Merged

Fix Batching Issues#1045
martindevans merged 2 commits intoSciSharp:masterfrom
martindevans:investigate_batching_issues

Conversation

@martindevans
Copy link
Copy Markdown
Member

In #1038 a new example was merged. However, this exposed a bug in the batching mechanism, sharing tokens when they shouldn't be shared. This PR completely removes the token sharing mechanism - tokens are only shared if the methods which explicitly add tokens to multiple sequences are used. While working on this I added some extra sanity checking to the batch, which I've left in place (DEBUG only).

I've also added more details explaining the BatchedExecutorSimple example, in particular explaining the NoKvSlot error which confuses a lot of people. @phil-scott-78 this may interest you :)

…causing problems with tokens that should never have been shared.

 - Added a helper to simplify sampling a conversation with a sampling pipeline
 - Added many more comments explaining BatchedExecutorSimple in detail
@phil-scott-78
Copy link
Copy Markdown
Contributor

Love it. New comments in the sample answered some questions I didn't even know I wanted to ask.

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.

2 participants