zhaih commented on code in PR #12050:
URL: https://github.com/apache/lucene/pull/12050#discussion_r1091236468
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene95/Lucene95HnswVectorsWriter.java:
##########
@@ -489,6 +485,220 @@ public void mergeOneField(FieldInfo fieldInfo, MergeState
mergeState) throws IOE
}
}
+ private HnswGraphBuilder<float[]> createFloatVectorHnswGraphBuilder(
Review Comment:
nit: can we merge this function with L519 using generic?
##########
lucene/core/src/java/org/apache/lucene/util/hnsw/HnswGraph.java:
##########
@@ -182,10 +201,43 @@ public int nextInt() {
public boolean hasNext() {
return cur < size;
}
+ }
- /** The number of elements in this iterator * */
- public int size() {
- return size;
+ /** Nodes iterator based on set representation of nodes. */
+ public static class SetNodesIterator extends NodesIterator {
Review Comment:
nit: I feel like this can be some more general name, since the only
requirement is that the input provides a int iterator. But anyway this
shouldn't block the PR.
##########
lucene/core/src/java/org/apache/lucene/util/hnsw/OnHeapHnswGraph.java:
##########
@@ -33,44 +32,36 @@
public final class OnHeapHnswGraph extends HnswGraph implements Accountable {
private int numLevels; // the current number of levels in the graph
- private int entryNode; // the current graph entry node on the top level
+ private int entryNode; // the current graph entry node on the top level. -1
if not set
- // Nodes by level expressed as the level 0's nodes' ordinals.
- // As level 0 contains all nodes, nodesByLevel.get(0) is null.
- private final List<int[]> nodesByLevel;
-
- // graph is a list of graph levels.
- // Each level is represented as List<NeighborArray> – nodes' connections on
this level.
+ // Level 0 is represented as List<NeighborArray> – nodes' connections on
level 0.
// Each entry in the list has the top maxConn/maxConn0 neighbors of a node.
The nodes correspond
// to vectors
// added to HnswBuilder, and the node values are the ordinals of those
vectors.
// Thus, on all levels, neighbors expressed as the level 0's nodes' ordinals.
- private final List<List<NeighborArray>> graph;
+ private final List<NeighborArray> graphLevel0;
+ // Represents levels 1-N. Each level is represented with a TreeMap that maps
a levels level 0
+ // ordinal to its
+ // neighbors on that level.
+ private final List<TreeMap<Integer, NeighborArray>> graphUpperLevels;
Review Comment:
I think to maintain the index we're still adding a fake "level 0" as `null`,
can we reflect that in the comment? (By explicitly state that we'll always
insert a `null` to fill level 0's index?)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
