Upgrade bf-tree dependency#1183
Conversation
There was a problem hiding this comment.
Pull request overview
This PR upgrades the bf-tree dependency to 0.5.x and migrates DiskANN’s bf-tree provider persistence from the old snapshot APIs to the new CPR snapshot APIs, including updating benchmark inputs/examples and adding round-trip save/load coverage.
Changes:
- Bump
bf-treefrom0.4.xto0.5.xand migrate provider save/load to CPR snapshots. - Introduce
use_snapshotplumbing across provider parameters, saved params, and benchmark JSON inputs/examples. - Add benchmark-driven round-trip save/load tests for bf-tree full-precision and spherical (quantized) indexes.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| diskann-bftree/src/provider.rs | CPR snapshot save/load migration; new use_snapshot flag added to provider + serialization. |
| diskann-bftree/src/neighbors.rs | Update neighbor provider snapshot test to use cpr_snapshot. |
| diskann-benchmark/src/main.rs | Add bf-tree benchmark round-trip save/load integration tests. |
| diskann-benchmark/src/inputs/bftree.rs | Add use_snapshot to bf-tree benchmark store config and map into provider params. |
| diskann-benchmark/src/index/bftree/full_precision.rs | Save built bf-tree index when save_path is requested. |
| diskann-benchmark/src/index/bftree/spherical.rs | Save built bf-tree spherical index when save_path is requested. |
| diskann-benchmark/example/graph-index-bftree.json | Add use_snapshot field to example store configs. |
| diskann-benchmark/example/graph-index-bftree-stream.json | Add use_snapshot field to example store configs. |
| diskann-benchmark/example/graph-index-bftree-spherical.json | Add use_snapshot field to example store configs (incl. quant). |
| Cargo.toml | Update dependency requirement to bf-tree = "0.5". |
| Cargo.lock | Lockfile update to bf-tree 0.5.4 and dependency graph adjustments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Ok(Self { | ||
| quant_vectors: quant_precursor.create(params.quant_vector_provider_config)?, | ||
| full_vectors: VectorProvider::new_with_config( | ||
| params.max_points, | ||
| params.dim, | ||
| params.num_start_points.get(), | ||
| params.vector_provider_config, | ||
| )?, | ||
| neighbor_provider: NeighborProvider::new_with_config( | ||
| params.max_degree, | ||
| params.neighbor_list_provider_config, | ||
| )?, | ||
| metric: params.metric, | ||
| graph_params: params.graph_params, | ||
| use_snapshot: params.use_snapshot, | ||
| }) |
| use_snapshot: vector_store_config | ||
| .as_ref() | ||
| .and_then(|c| c.use_snapshot) | ||
| .unwrap_or(false), |
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (89.33%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1183 +/- ##
==========================================
- Coverage 89.47% 89.46% -0.02%
==========================================
Files 486 487 +1
Lines 92161 92177 +16
==========================================
+ Hits 82458 82462 +4
- Misses 9703 9715 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- Update bf-tree dependency from 0.4.9 to 0.5 - Migrate save/load to cpr_snapshot/new_from_cpr_snapshot - Add use_snapshot field to BfTreeProvider, BfTreeProviderParameters, SavedParams, and BfTreeStoreConfig - Guard save_bftree against unconfigured snapshot support - Move record size validation into provider constructors (from PR #1166) - Remove dead BfTreeParams::to_config and copy_snapshot_if_needed - Add save_path support and round-trip tests for both full-precision and spherical (quantized) bftree indexes - Remove #[ignore] on spherical round-trip test (0.5 fixes the panic) - Require use_snapshot in benchmark JSON input configs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove top-level use_snapshot from build structs; derive from store configs - Add reconcile_use_snapshot() to validate all configs agree - Make BfTreeStoreConfig::use_snapshot a required bool (not Option) - Force-set use_snapshot on all bf-tree Configs in provider constructor - bftree_parameters_from now returns anyhow::Result Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
06abe40 to
3902c6a
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@JordanMaples - I was testing this PR for save_path and found save_path is not working for graph-index-bftree-stream.json benchmark. |
Bump bf-tree 0.4 → 0.5
Upgrades the bf-tree dependency from 0.4.9 to 0.5.4 and migrates to the new CPR snapshot API.
Changes
Snapshot API migration
use_snapshot configuration
Round-trip tests
Benchmark results (wikipedia-100K, float32, inner product)