search

nasa-jpl / autoRIFT

A Python module of a fast and intelligent algorithm for finding the pixel displacement between two images
Apache License 2.0
212 stars 52 forks source link

opencv-highgui dependency not needed? #53

Closed rtburns-jpl closed 2 years ago

rtburns-jpl commented 2 years ago

Quick question, is the opencv highgui module not actually needed?

I was able to remove this include and still build isce2's autoRIFT module without any errors:

https://github.com/nasa-jpl/autoRIFT/blob/611ff6921c9d989d6b14c52927c9afd44caabe37/geo_autoRIFT/autoRIFT/bindings/autoriftcoremodule.cpp#L45

Removing this would slim down the dependencies a bit, as we'd then only need the core and imgproc modules.

alex-s-gardner commented 2 years ago

@leiyangleon is there anything that we need highgui for?

jhkennedy commented 2 years ago

@alex-s-gardner and @rtburns-jpl I suspect it's not needed as its to visualize results in the opencv framework: https://docs.opencv.org/3.4/d7/dfc/group__highgui.html

Likely was used/useful for development, but isn't needed for releases/production.

jhkennedy commented 2 years ago

@rtburns-jpl if you have a dev build with dependencies removed pushed to conda-forge or some channel, I can run it in our pipelines and compare the results.

rtburns-jpl commented 2 years ago

@jhkennedy conda doesn't break up the opencv into separate packages, but Ubuntu does. I noticed this while working on https://github.com/isce-framework/isce2/pull/442, but that only tests the isce2 module part of autoRIFT.

jhkennedy commented 2 years ago

@rtburns-jpl ah yeah, everything we do is based on the conda-forge package. I suspect you'd be fine removing it.

leiyangleon commented 2 years ago

@rtburns-jpl @jhkennedy @alex-s-gardner Although I am not 100% sure where it got used, if you google "opencv2 highgui.hpp matchTemplate", you would find many examples of including it for opencv2's matchTemplate function, which is used for autoRIFT's core NCC component. I would keep it if not causing conflicts with other dependencies. Also, before removing it, it would be good to verify the netcdf output(s) on a pixel-by-pixel basis to make sure we don't deteriorate the accuracy.