jimczi commented on code in PR #12246:
URL: https://github.com/apache/lucene/pull/12246#discussion_r1179349185
##########
lucene/analysis/common/src/java/org/apache/lucene/analysis/synonym/word2vec/Word2VecSynonymProvider.java:
##########
@@ -64,7 +65,15 @@ public Word2VecSynonymProvider(Word2VecModel model) throws
IOException {
this.hnswGraph = builder.build(word2VecModel.copy());
}
- public List<TermAndBoost> getSynonyms(
+ /**
+ * Returns the list of synonyms of a provided term. This method is
synchronized because it uses
+ * the {@link org.apache.lucene.util.hnsw.OnHeapHnswGraph} that is not
thread-safe.
+ *
+ * @param term term to search to find synonyms
+ * @param maxSynonymsPerTerm limit of synonyms returned
+ * @param minAcceptedSimilarity lower similarity threshold to consider
another term as synonym
+ */
+ public synchronized List<TermAndBoost> getSynonyms(
Review Comment:
Sure that works but I wouldn't call that a solution to be honest. I am again
raising the usability here. Currently this filter has a very high loading cost
and this change would add thread contentions on a non-trivial task (searching
the graph). As I said in the previous PR, it would be preferable to add an
offline path that builds a static graph. It's also possible to generate the
synonym rules offline by performing the knn search on the entire vocabulary.
The advantage of such approach is that the existing synonym filter could be
reuse.
I don't see how the current approach would be usable in a real world
scenario.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
