jimczi commented on code in PR #12246:
URL: https://github.com/apache/lucene/pull/12246#discussion_r1179707991
##########
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:
My concern was more that the feature is exposed in the analysis-common
package and considering the current limitations it doesn't feel common to me.
I'd be less pedantic, if that's the good term, if it was added in the sandbox
as an experimentation where we can iterate more freely. In any case don't
consider my comment as a veto to merge, please proceed with what you think is
best as long as the issue around multi-threading is solved.
--
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
