[GitHub] [lucene] msokolov commented on pull request #12248: use HashMap instead of TreeMap for OnHeapHnswGraph neighbors

msokolov commented on PR #12248: URL: https://github.com/apache/lucene/pull/12248#issuecomment-1528781897 I'm not entirely sure why just from inspection, but this seems to have broken some of the backwards-compatibility tests. What it means is this can no longer read indexes written by a pr

[GitHub] [lucene] msokolov commented on a diff in pull request #12248: use HashMap instead of TreeMap for OnHeapHnswGraph neighbors

msokolov commented on code in PR #12248: URL: https://github.com/apache/lucene/pull/12248#discussion_r1181072749 ## lucene/core/src/test/org/apache/lucene/util/hnsw/HnswGraphTestCase.java: ## @@ -265,19 +265,34 @@ public KnnVectorsFormat getKnnVectorsFormatForField(String field

[GitHub] [lucene] jbellis commented on pull request #12248: use HashMap instead of TreeMap for OnHeapHnswGraph neighbors

jbellis commented on PR #12248: URL: https://github.com/apache/lucene/pull/12248#issuecomment-1528877184 I've added the types in, but I'd like to push back a bit on getting rid of requiring Locale for string.formatted in test code -- the alternative isn't really using Locale, the alternativ

[GitHub] [lucene] jbellis commented on pull request #12248: use HashMap instead of TreeMap for OnHeapHnswGraph neighbors

jbellis commented on PR #12248: URL: https://github.com/apache/lucene/pull/12248#issuecomment-1528878444 ^ I think that's everything, I'll look at what's going on w/ the tests for the NeighborQueue piece and create a new PR -- This is an automated message from the Apache Git Service. To r

[GitHub] [lucene] jbellis opened a new pull request, #12255: allocate one NeighborQueue per search for results

jbellis opened a new pull request, #12255: URL: https://github.com/apache/lucene/pull/12255 As discussed on the hashmap PR, this saves about 1% of graph build time. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use t

[GitHub] [lucene] jbellis commented on pull request #12255: allocate one NeighborQueue per search for results

jbellis commented on PR #12255: URL: https://github.com/apache/lucene/pull/12255#issuecomment-1528882157 @msokolov wrote earlier, > I'm not entirely sure why just from inspection, but this seems to have broken some of the backwards-compatibility tests. What it means is this can no lo

[GitHub] [lucene] jbellis commented on pull request #12255: allocate one NeighborQueue per search for results

jbellis commented on PR #12255: URL: https://github.com/apache/lucene/pull/12255#issuecomment-1528882996 hmm, this is interesting, the hashmap branch passes tests, the neighborqueue branch passes tests, but putting them together does not pass. investigating... -- This is an automated mes

[GitHub] [lucene] jbellis commented on pull request #12248: use HashMap instead of TreeMap for OnHeapHnswGraph neighbors

jbellis commented on PR #12248: URL: https://github.com/apache/lucene/pull/12248#issuecomment-1528887917 ^ last commit addresses the legacy test failures, not clear to me why they didn't fail before on this branch, but with this change everything passes with and w/o the NQ commit -- This

[GitHub] [lucene] jbellis commented on pull request #12255: allocate one NeighborQueue per search for results

jbellis commented on PR #12255: URL: https://github.com/apache/lucene/pull/12255#issuecomment-1528887975 ^ this is addressed on the hashmap PR now -- 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

[GitHub] [lucene] zhaih commented on a diff in pull request #12246: Set word2vec getSynonyms method synchronized

zhaih commented on code in PR #12246: URL: https://github.com/apache/lucene/pull/12246#discussion_r1181173636 ## lucene/analysis/common/src/java/org/apache/lucene/analysis/synonym/word2vec/Word2VecSynonymProvider.java: ## @@ -64,7 +65,15 @@ public Word2VecSynonymProvider(Word2Ve