You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "javanna (via GitHub)" <gi...@apache.org> on 2023/01/24 22:41:13 UTC
[GitHub] [lucene] javanna opened a new pull request, #12111: SimpleText codec to support writing byte vectors
javanna opened a new pull request, #12111:
URL: https://github.com/apache/lucene/pull/12111
A recent test failure signaled that when the simple text codec was randomly selected, byte vectors could not be written. This commit addressed that by adding support for writing byte vectors to SimpleTextKnnVectorsWriter.
Note that while support is added to the BufferingKnnVectorsWriter base class, 90, 91 and 92 writers don't need to support byte vectors and will throw unsupported operation exception when attempting to do that.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] javanna merged pull request #12111: SimpleText codec to support writing byte vectors
Posted by "javanna (via GitHub)" <gi...@apache.org>.
javanna merged PR #12111:
URL: https://github.com/apache/lucene/pull/12111
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] benwtrent commented on a diff in pull request #12111: SimpleText codec to support writing byte vectors
Posted by "benwtrent (via GitHub)" <gi...@apache.org>.
benwtrent commented on code in PR #12111:
URL: https://github.com/apache/lucene/pull/12111#discussion_r1086603790
##########
lucene/core/src/java/org/apache/lucene/codecs/BufferingKnnVectorsWriter.java:
##########
@@ -161,77 +161,113 @@ public int advance(int target) throws IOException {
}
}
+ /** Sorting FloatVectorValues that iterate over documents in the order of the provided sortMap */
Review Comment:
```
/** Sorting SortingByteVectorValues that iterate over documents in the order of the provided sortMap */
```
Comment is incorrect. But not a huge deal.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] javanna commented on a diff in pull request #12111: SimpleText codec to support writing byte vectors
Posted by "javanna (via GitHub)" <gi...@apache.org>.
javanna commented on code in PR #12111:
URL: https://github.com/apache/lucene/pull/12111#discussion_r1086409971
##########
lucene/core/src/java/org/apache/lucene/codecs/BufferingKnnVectorsWriter.java:
##########
@@ -39,79 +37,81 @@
* @lucene.experimental
*/
public abstract class BufferingKnnVectorsWriter extends KnnVectorsWriter {
- private final List<FieldWriter> fields = new ArrayList<>();
+ private final List<FieldWriter<?>> fields = new ArrayList<>();
/** Sole constructor */
protected BufferingKnnVectorsWriter() {}
@Override
- public KnnFieldVectorsWriter<float[]> addField(FieldInfo fieldInfo) throws IOException {
- FieldWriter newField = new FieldWriter(fieldInfo);
+ public KnnFieldVectorsWriter<?> addField(FieldInfo fieldInfo) throws IOException {
Review Comment:
yes that's a good point and we should probably look at #11758 as a whole and see what we can improve further.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] jpountz commented on a diff in pull request #12111: SimpleText codec to support writing byte vectors
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on code in PR #12111:
URL: https://github.com/apache/lucene/pull/12111#discussion_r1086310610
##########
lucene/core/src/java/org/apache/lucene/codecs/BufferingKnnVectorsWriter.java:
##########
@@ -39,79 +37,81 @@
* @lucene.experimental
*/
public abstract class BufferingKnnVectorsWriter extends KnnVectorsWriter {
- private final List<FieldWriter> fields = new ArrayList<>();
+ private final List<FieldWriter<?>> fields = new ArrayList<>();
/** Sole constructor */
protected BufferingKnnVectorsWriter() {}
@Override
- public KnnFieldVectorsWriter<float[]> addField(FieldInfo fieldInfo) throws IOException {
- FieldWriter newField = new FieldWriter(fieldInfo);
+ public KnnFieldVectorsWriter<?> addField(FieldInfo fieldInfo) throws IOException {
Review Comment:
Unrelated: your change reminds me that I'd rather like to split this method in two: `KnnFieldVectorsWriter<float[]> addFloatField` and `KnnFieldVectorsWriter<byte[]> addByteField`. Otherwise we can never take advantage of the type safety of generics? This is a codec internal rather than something user-facing so we can look into it after 9.5.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org