Skip to content

Fix save_path silently ignored in bftree benchmarks#1163

Closed
Copilot wants to merge 2 commits into
mainfrom
copilot/save-path-bug-fix
Closed

Fix save_path silently ignored in bftree benchmarks#1163
Copilot wants to merge 2 commits into
mainfrom
copilot/save-path-bug-fix

Conversation

Copilot AI commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

save_path in graph-index-bftree and graph-index-bftree-spherical JSON configs had no effect: both BfTreeFullPrecision and BfTreeSpherical benchmark backends built the index but never persisted it.

Changes

  • bftree/full_precision.rs — after run_build + checkpoint, call save_with on the provider when save_path is set
  • bftree/spherical.rs — same, after single-insert build + checkpoint

Both follow the existing pattern in the non-bftree FullPrecision benchmark. BfTreeProvider already implements SaveWith<String> for both NoStore and QuantVectorProvider — it just wasn't being called.

// save the index if requested
if let Some(save_path) = input.build().save_path() {
    tokio::block_on(
        index
            .provider()
            .save_with(&FileStorageProvider, &save_path.to_string()),
    )?;
}

The BfTreeFullPrecision and BfTreeSpherical benchmarks did not call
save_with on the provider after building the index, so the save_path
field in the JSON input was silently ignored.

Add the save logic (mirroring the non-bftree FullPrecision benchmark)
to both bftree benchmark run() methods:
- diskann-benchmark/src/backend/index/bftree/full_precision.rs
- diskann-benchmark/src/backend/index/bftree/spherical.rs

Closes #1157
Copilot AI changed the title [WIP] Fix save_path not working in graph-index-bftree Fix save_path silently ignored in bftree benchmarks Jun 13, 2026
Copilot AI requested a review from harsha-simhadri June 13, 2026 21:35
@JordanMaples

Copy link
Copy Markdown
Contributor

I'll pick this up on a new branch. In my local testing of this, there were some problems with saving the quantized tree. It looks like by upgrading to the 0.5.x versions of bftree, this might be sorted.

@JordanMaples

Copy link
Copy Markdown
Contributor

Closing this in favor of #1183

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