[request] Suggesting a list of minor changes

[C-H-Chien]

Hi! I have a few suggestions that has been implemented in my forked repo:

• Since there is no timer even though there is a --print-time-info option, I added a timer using cudaEventElapsedTime to measure the GPU runtime, both in main.cpp and match.cpp. I can also add the timer for the CPU side on loading the images, writing data to a file, etc.
• I am running the code on a server which does not have a DevIL support and I do not have the authority to the root access. Thus, I added OpenCV support that reads the input image. This is optional: if there is no OpenCV, then it falls back to the current implementation.
• Feature orientations are written to the file output-features.txt instead of 0 which is currently a placeholder.
• The feature matching results are transferred from the GPU memory to the CPU memory so that they can be written to a file.
• The README file is updated to include how a new user can do a quick try. Honestly I spent some time trying to figure out where the execution files are, what command should I give, etc. A clear instruction would be very helpful.

I can open a pull request to address the above changes, but would like to hear if you have any thoughts. Thanks!

[griwodz]

Hi,

Thank you for your proposed changes!
I look forward to your PR(s)

(1) Great that you revived the timer. After development was finished, it wasn't a great tool any more because it was good for testing the Descriptor extraction time, but loading images from disk and decoding them is so slow that the reported numbers for SIFT speed were misleading. But perhaps you report with and without the image transformation?

(2) DeVIL is actually quite bad. I didn't want an OpenCV-dependency because it is so huge and has no many sub-dependencies. If you can make it optional in CMake, I'm happy to have it (instead of DeVIL or as a CMake choice). There is also Commit 191daea from the wip/prep-for-sycl branch that uses OpenImageIO instead of DeVIL.

(3) There is also code for orientation output in the fix/vlFeatCompliance3 branch, and probably a couple of other branches. I couldn't get anyone to accept those changes yet. I'm not sure if it's exactly as you need it.

(4) The fix/vlFeatCompliance3 branch has that as well :-(
I added a method matchAndReturn for this and used cudaManagedMemory

(5) Great with the improved README.

[C-H-Chien]

Thank you so much for the feedback! I found that there is some issue in constructing feature correspondences when the data is transferred from the GPU memory to the CPU memory. I will fix that before opening a PR.

(1) I think reporting both with and without image transformation is a great idea.

(2) Yes, my implementation makes OpenCV dependency optional to the users. It falls back to the current version if OpenCV is not available.

(3) I can check the branch fix/vlFeatCompliance3. Basically it is just returning the orientation. I can further verify if the returned orientation is correct or not.

(4) Thanks! I guess fix/vlFeatCompliance3 branch is what I am aiming for. Perhaps I can merge that branch with mine to avoid duplicate contribution.

Will open a PR when all is done. Thank you!