fix hnsw param use #1695
fix hnsw param use #1695
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1695 +/- ##
==========================================
- Coverage 82.09% 82.07% -0.03%
==========================================
Files 333 333
Lines 19400 19400
==========================================
- Hits 15927 15923 -4
- Misses 3473 3477 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -28,7 +28,7 @@ pub fn level_factor() -> f64 { | |||
|
|
|||
| /// Upper limit to the number of out-edges a embedding can have. | |||
| pub const fn m_max() -> usize { | |||
| 30 | |||
| 60 | |||
There was a problem hiding this comment.
What implies increasing this number?
(doubt: why are we using a const fn instead of a const variable?)
There was a problem hiding this comment.
It has several implications both in terms of space and read-write speed. To learn more about its implications I suggest looking at hnsw paper, in particular to the insert algorithm. For our case, we had m and m_max set to the same number, which makes insertion slower (as can be inferred by looking at the algorithm). From what I saw defining m_max as 2*m when m is small is a good approach.
As for why is a const fn and not a constant, I did it for uniformity's sake, since level_factor is a function. That said, I do not mind moving it to a constant.
hermeGarcia
force-pushed
the
hnsw-better-param-use
branch
from
2280f1f
to
eae6515
Compare
Description
Describe the proposed changes made in this PR.
How was this PR tested?
Describe how you tested this PR.