jimczi commented on PR #12314:
URL: https://github.com/apache/lucene/pull/12314#issuecomment-1564665498

   > nothing in this PR is final nor I have any strong opinion about it.
   
   Sure, we're just discussing the approach, no worries.
   
   > In regards to your main worry, can you point me to the areas of code you 
don't like specifically and I can have a thought in how to modify them!
   
   This change in [Float|Byte]VectorValues:
   
   ```
    /** The maximum length of a vector */
     public static final int MAX_DIMENSIONS = 1024;
   
     protected boolean multiValued = false;
   
     public boolean isMultiValued() {
       return multiValued;
     }
   
     /** Sole constructor */
     protected FloatVectorValues() {}
   
   @@ -57,4 +63,13 @@ public final long cost() {
      * @return the vector value
      */
     public abstract float[] vectorValue() throws IOException;
   
     /**
      * Return the document ID corresponding to the input vector id(ordinal)
      * @param ord vector ID(ordinal)
      * @return the document ID
      */
     public int ordToDoc(int ord){
       return ord;
     }
   }
   ```
   
   Today the expectation is that it iterates over doc ids. This change adds an 
indirection that needs to be checked (`isMultivalued`) and if it's the case 
then the doc id is an ordinal id that needs to be transformed with `ordToDoc` .
   That's trappy, I am not even sure how you can advance to a doc rather than 
an ordinal and the code would mean different things based on whether you're 
working on multivalued or not. Considering how the original APIs was made for 
single valued I don't think we should try to sneak multi-valued into the model. 
That's why I propose that we add it as separated like you originally did. It's 
a doc values + search APIs so it needs to be usable for these two purposes in a 
more predictive way.  
   
   
   
   
   
   
   
   


-- 
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

Reply via email to