Skip to content

add enable brute force search - #7

Open
c-i-njoroge wants to merge 5 commits into
mainfrom
charles-njoroge/knowhere-hnsw-brute-force
Open

add enable brute force search#7
c-i-njoroge wants to merge 5 commits into
mainfrom
charles-njoroge/knowhere-hnsw-brute-force

Conversation

@c-i-njoroge

Copy link
Copy Markdown
Collaborator

💸 TL;DR

Allow us to enable brute force searches so we can measure recall against the source of truth

📜 Details

Design Doc

Jira

🧪 Testing Steps / Validation

✅ Checks

  • CI tests (if present) are passing
  • Adheres to code style for repo
  • Contributor License Agreement (CLA) completed if not a Reddit employee

@c-i-njoroge
c-i-njoroge requested a review from hicder-rddt August 4, 2026 21:38
@c-i-njoroge
c-i-njoroge marked this pull request as ready for review August 4, 2026 21:38

@hicder-rddt hicder-rddt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

small nit, but LGTM


// an explicit user request to force brute force overrides the heuristics below
const auto* hnsw_cfg = dynamic_cast<const BaseHnswConfig*>(&cfg);
if (hnsw_cfg != nullptr && hnsw_cfg->force_brute_force.has_value() && hnsw_cfg->force_brute_force.value()) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (hnsw_cfg != nullptr && hnsw_cfg->force_brute_force.has_value() && hnsw_cfg->force_brute_force.value()) {
if (hnsw_cfg != nullptr && hnsw_cfg->force_brute_force.value_or(true)) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Wouldn't we want this to be false? Basically if the field doesn't exist then the user did not intend to enable brute force

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

oh yeah I mean false

Comment on lines +45 to +46
LOG_KNOWHERE_INFO_ << "force_brute_force enabled: bypassing HNSW graph traversal for exhaustive search (k="
<< cfg.k.value() << ", ntotal=" << index->ntotal << ")";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we may not want to log info for every request. maybe debug?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is only when brute force is enabled with the flag. I can set to debug after some more testing

Comment on lines +1471 to +1473
LOG_KNOWHERE_INFO_ << "force_brute_force: completed exhaustive search for " << rows
<< " queries (k=" << k << ", ntotal=" << indexes[index_id]->ntotal
<< ", used_bf_wrapper=" << whether_bf_search.value_or(false) << ")";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

maybe debug? but up to you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants