Skip to content

Set default batch size to 16 - #267

Merged
anasdorbani merged 3 commits into
dais-polymtl:devfrom
anasdorbani:batch-default-size-16
Jun 3, 2026
Merged

Set default batch size to 16#267
anasdorbani merged 3 commits into
dais-polymtl:devfrom
anasdorbani:batch-default-size-16

Conversation

@anasdorbani

Copy link
Copy Markdown
Member

No description provided.

Copilot AI review requested due to automatic review settings June 3, 2026 03:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR standardizes the model default batch_size to 16 across the extension by introducing a shared constant and using it when model configuration doesn’t specify a batch size. It also simplifies llm_rerank’s batching logic to always clamp the batch size to the number of tuples, and updates unit tests accordingly.

Changes:

  • Introduce DEFAULT_BATCH_SIZE = 16 as a shared constant in the model manager public header.
  • Use DEFAULT_BATCH_SIZE when a model’s batch_size is not provided via config/DB args.
  • Simplify llm_rerank sliding-window batching by clamping batch_size to num_tuples, and update unit tests to match the new default.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
test/unit/model_manager/model_manager_test.cpp Updates the minimal initialization test to assert the new default batch size constant.
src/model_manager/model.cpp Replaces the hardcoded default batch size with DEFAULT_BATCH_SIZE when missing from config/DB args.
src/include/flock/model_manager/repository.hpp Adds DEFAULT_BATCH_SIZE constant (16) for shared use across the codebase.
src/functions/aggregate/llm_rerank/implementation.cpp Removes special-casing tied to the old default and always clamps batch size to tuple count.

@anasdorbani
anasdorbani force-pushed the batch-default-size-16 branch from a6659ce to 75a25b9 Compare June 3, 2026 03:51
if (batch_size == 2048) {
batch_size = std::min<int>(batch_size, num_tuples);
}
auto batch_size = std::min<int>(model.GetModelDetails().batch_size, num_tuples);

@queryproc queryproc Jun 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should make sure that we take the min of 2: (user_batch, default_batch = 16). Using this batch size, we then expand the data section of the prompt while not going beyond the max input context window size.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Alternatively, if the user provided a batch_size, it overrides the default size and then we expand till we reach the max input context window size.

@anasdorbani
anasdorbani merged commit 2f2b1c2 into dais-polymtl:dev Jun 3, 2026
1 of 5 checks passed
lhoestq pushed a commit to lhoestq/flock that referenced this pull request Jul 8, 2026
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