msokolov commented on PR #16567: URL: https://github.com/apache/lucene/pull/16567#issuecomment-5441777282
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. -- 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]
