opencv icon indicating copy to clipboard operation
opencv copied to clipboard

Add interface to Annoy which will replace the FLANN

Open kaingwade opened this issue 1 year ago • 2 comments

This PR is to add interface to Annoy which will replace the FLANN, part of one of the cleanup work of OpenCV 5.0: #24998.

After it, there will be consecutive patches:

  • [ ] Add Annoy based DescriptorMatcher
  • [ ] Replace FLANN based code with Annoy and remove FLANN completely

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • [x] I agree to contribute to the project under Apache 2 License.
  • [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • [x] The PR is proposed to the proper branch
  • [ ] There is a reference to the original bug report and related work
  • [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name.
  • [ ] The feature is well documented and sample code can be built with the project CMake

kaingwade avatar Jun 05 '24 04:06 kaingwade

I propose to replace KISS random generator with our own RNG.

asmorkalov avatar Sep 06 '24 13:09 asmorkalov

I propose to not touch Annoy itself, but implement simple adapter on top of RNG. In long term, I prefer to have vanilla Annoy instead of patched to have a chance to update it without pain. Also please add license file to Annoy folder and add ocv_install_3rdparty_licenses to cmake to include license information to binary distributions.

asmorkalov avatar Sep 23 '24 08:09 asmorkalov

@asmorkalov, regarding Random, RNG, theRNG. Random is now based on RNG — this is fine. Using the global theRNG() is dangerous. It means that depending on when Annoy structure is created, behaviour will be different. Tests will not catch it, because they reset theRNG(), but in real applications using global RNG is rather a problem than benefit

vpisarev avatar Oct 24 '24 08:10 vpisarev