Skip to content

Fix alpha pruning documentation - #1351

Open
xinyuwen2 wants to merge 1 commit into
mainfrom
wxy/fix-alpha-pruning-comments
Open

Fix alpha pruning documentation#1351
xinyuwen2 wants to merge 1 commit into
mainfrom
wxy/fix-alpha-pruning-comments

Conversation

@xinyuwen2

Copy link
Copy Markdown
Contributor
  • Does this PR have a descriptive title that could go in our release notes?
  • Does this PR add any new dependencies?
  • Does this PR modify any existing APIs?
  • Is the change to the API backwards compatible?
  • Should this result in any changes to our documentation, either updating existing docs or adding new ones?

Reference Issues/PRs

What does this implement/fix? Briefly explain your changes.

Corrects comments describing alpha during robust pruning. Higher alpha values make pruning less aggressive and generally retain more edges, producing denser graphs.

Any other comments?

Documentation-only change; no runtime behavior is modified.

@xinyuwen2
xinyuwen2 requested review from a team and a lite review from Copilot August 24, 2026 08:10

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

Updates DiskANN graph pruning documentation to correct how the alpha parameter affects robust pruning behavior, aligning the rustdoc comments with the pruning implementation semantics.

Changes:

  • Clarifies that higher alpha generally makes pruning less aggressive and yields denser graphs.
  • Updates alpha builder method documentation to match the corrected meaning.
  • Refines explanatory text around pruning behavior in the PruneKind rustdoc.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@@ -51,8 +51,8 @@ use crate::utils::IntoUsize;
/// entirely as a neighbor candidate (achieved by settings its occlusion factor to
@@ -51,8 +51,8 @@ use crate::utils::IntoUsize;
/// entirely as a neighbor candidate (achieved by settings its occlusion factor to
/// `f32::MAX`.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.55%. Comparing base (860cf47) to head (325501a).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1351      +/-   ##
==========================================
- Coverage   91.55%   91.55%   -0.01%     
==========================================
  Files         521      521              
  Lines      100347   100347              
==========================================
- Hits        91877    91870       -7     
- Misses       8470     8477       +7     
Flag Coverage Δ
miri 91.55% <ø> (-0.01%) ⬇️
unittests 91.23% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
diskann/src/graph/config/mod.rs 98.98% <ø> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

Thanks! Please wait for Magdalen Dobson Manohar (@magdalendobson) to sign-off.

/// A higher occlusion factor means that `j` and `k` are "more similar" than `i` and `k`.
/// The pruning rules are heuristics established such that using higher values of `alpha`
/// yields sparser graphs.
/// makes pruning less aggressive and generally yields denser graphs.

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.

Magdalen Dobson Manohar (@magdalendobson) - can you quickly sanity check this as well. I was the one who got it wrong in the first place, so someone other than me should eyeball it :laugh:

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.

5 participants