RKSPD commented on PR #16567: URL: https://github.com/apache/lucene/pull/16567#issuecomment-5443288699
> exciting! the numbers you shared look compelling. Can you address the failing checks? It looks like a reference to the new module crept in to another module somehow? And there are some warnings that need to be cleaned up. But beyond that, this is a _lot_ of new code to review. I wonder if you can help reviewers by (1) providing some overview of the algorithm and the new classes. If you included that in the PR javadocs or module.java, that's ideal, but please reference from the PR description if so. Also (2) is there any code here that's optional for a first implementation? For example I see you included multiple quantization options. Can we split this up and look at a stripped down version without quantization support? I'm afraid this may languish if we can't simplify it. Hi Mike! Thank you for commenting on my PR. I agree this is really huge and hard to review. I'm working on breaking the codec down into reviewable pieces, but just wanted to get the results out so the community can get a feel for the results and the implementation scope. Unlike other codecs, IVFaster necessarily uses a cascading coarse/fine quantizer in both the index and search phases, which makes it difficult to think about the IVFaster algorithm without these pieces. The performance characteristics are largely based on the new quantizer design. I'll get more comprehensive documentation and architecture diagrams soon. Once again thank you. -- 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]
