autoRift does not work with OpenCV 4.x

[hfattahi]

As pointed out by @vbrancat in #164 and @EJFielding in #169 , autoRift currently only builds with OpenCV 3.x . autoRift needs to be upgraded to be consistent with openCV 4.x.

[piyushrpt]

This is probably an autoRIFT ticket. Their requirements clearly state opencv 3.x for now.

[hfattahi]

Since autoRift is currently building within isce2 then I think this is still an issue and we need to address it. Possible solutions: 1- fix autoRift 2- make autoRift build optional

[EJFielding]

I vote for option 2. I already lost many hours trying to get autoRIFT to install on my MacOS and CentOS computers.

[leiyangleon]

@hfattahi @piyushrpt @EJFielding @rtburns-jpl

I just searched the conversation @piyushrpt and I had last October on the OpenCV versioning problem. The current OpenCV version has been explicitly set to v3.4 in autoRIFT's conda install only because there is no Python variants of v4 in Macports, through which autoRIFT was installed back to then. If one is able to use conda to build either opencv3 or opencv4 with Python, autoRIFT should be able to work. Then we can safely remove the explicit opencv version (v3.4) in the conda install yaml file, which gives the error to @EJFielding on the Linux machine (with v4.4 installed).

FYI, this is not related to what #164 reported, as the use of the code in autoRIFT should not be affected between v3.4 and v4. I also used v3.4's cv2.filter2D in autoRIFT and never noticed any problem as #164 reported.

If there is no easy fix of adding the numpy include folder (e.g. checking the path and add it to CPPPATH), it is fine to make autoRIFT an optional build. Again, opencv version should not be a problem to autoRIFT.

[EJFielding]

@leiyangleon When I tried to build ISCE2 with autoRIFT using a Conda-installed OpenCV v4.4, it fails because it seems to require C++11. See the error message that I posted in #169.

[leiyangleon]

@EJFielding okay, I see. That seems annoying if C++11 is a prerequisite to install OpenCV v4. So in that sense, maybe it is good to stay with opencv 3.4 unless ISCE2 wants to upgrade to C++11 in the future.

[rtburns-jpl]

That should be fine since many other modules require C++11. Can you try again with OpenCV 4.4 and this extra line?

diff --git a/contrib/geo_autoRIFT/SConscript b/contrib/geo_autoRIFT/SConscript
index e49f703..97d9cad 100755
--- a/contrib/geo_autoRIFT/SConscript
+++ b/contrib/geo_autoRIFT/SConscript
@@ -22,6 +22,7 @@ import os
 Import('envcontrib')
 package = 'geo_autoRIFT'
 envgeoAutorift = envcontrib.Clone()
+envgeoAutorift.MergeFlags('-std=c++11')
 envgeoAutorift['PACKAGE'] = envcontrib['PACKAGE'] + '/' + package
 install = envcontrib['PRJ_SCONS_INSTALL'] + '/' + envgeoAutorift['PACKAGE']
 listFiles = ['__init__.py']

[EJFielding]

OK, I reinstalled OpenCV 4.4 and added that line to the SConscript file, and it worked much better but still generated a few errors:

Install file: "contrib/geo_autoRIFT/autoRIFT/include/autoriftcoremodule.h" as "/u/pez0/fielding/tools/ISCE2_test/build/isce/components/contrib/geo_autoRIFT/autoRIFT/include/autoriftcoremodule.h"
g++ -o /u/pez0/fielding/tools/ISCE2_test/build/isce/components/contrib/geo_autoRIFT/autoRIFT/bindings/autoriftcoremodule.os -c -std=c++11 -O2 -Wall -fPIC -m64 -fPIC -DNEEDS_F77_TRANSLATION -DF77EXTERNS_LOWERCASE_TRAILINGBAR -I/u/vento-r0/ericf/packages/Anaconda/anaconda3/include/python3.7m -I/u/vento-r0/ericf/packages/Anaconda/anaconda3/lib/python3.7/site-packages/numpy/core/include -I/u/vento-r0/ericf/packages/Anaconda/anaconda3/include -I/u/vento-r0/ericf/packages/Anaconda/anaconda3/include/opencv4 -I/usr/include -I/u/pez0/fielding/tools/ISCE2_test/build/isce/components/iscesys/ImageApi/include -I/u/pez0/fielding/tools/ISCE2_test/build/isce/components/iscesys/ImageApi/DataCaster/include -I/u/pez0/fielding/tools/ISCE2_test/build/isce/components/isceobj/LineAccessor/include -I/u/pez0/fielding/tools/ISCE2_test/build/isce/components/iscesys/StdOE/include -I/u/pez0/fielding/tools/ISCE2_test/build/isce/components/isceobj/Util/include -I/u/pez0/fielding/tools/ISCE2_test/build/isce/components/isceobj/Util/Library/include -I/u/pez0/fielding/tools/ISCE2_test/build/isce/components/contrib/geo_autoRIFT/autoRIFT/include /u/pez0/fielding/tools/ISCE2_test/build/isce/components/contrib/geo_autoRIFT/autoRIFT/bindings/autoriftcoremodule.cpp
In file included from /u/vento-r0/ericf/packages/Anaconda/anaconda3/lib/python3.7/site-packages/numpy/core/include/numpy/ndarraytypes.h:1832:0,
                 from /u/vento-r0/ericf/packages/Anaconda/anaconda3/lib/python3.7/site-packages/numpy/core/include/numpy/ndarrayobject.h:12,
                 from /u/vento-r0/ericf/packages/Anaconda/anaconda3/lib/python3.7/site-packages/numpy/core/include/numpy/arrayobject.h:4,
                 from /u/pez0/fielding/tools/ISCE2_test/build/isce/components/contrib/geo_autoRIFT/autoRIFT/bindings/autoriftcoremodule.cpp:42:
/u/vento-r0/ericf/packages/Anaconda/anaconda3/lib/python3.7/site-packages/numpy/core/include/numpy/npy_1_7_deprecated_api.h:17:2: warning: #warning "Using deprecated NumPy API, disable it with " "#define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION" [-Wcpp]
 #warning "Using deprecated NumPy API, disable it with " \
  ^
/u/pez0/fielding/tools/ISCE2_test/build/isce/components/contrib/geo_autoRIFT/autoRIFT/bindings/autoriftcoremodule.cpp: In function ‘PyObject* arPixDisp_u(PyObject*, PyObject*)’:
/u/pez0/fielding/tools/ISCE2_test/build/isce/components/contrib/geo_autoRIFT/autoRIFT/bindings/autoriftcoremodule.cpp:142:50: error: ‘CV_TM_CCOEFF_NORMED’ was not declared in this scope
     cv::matchTemplate( my_imgR, my_imgC, result, CV_TM_CCOEFF_NORMED );
                                                  ^
/u/pez0/fielding/tools/ISCE2_test/build/isce/components/contrib/geo_autoRIFT/autoRIFT/bindings/autoriftcoremodule.cpp: In function ‘PyObject* arSubPixDisp_u(PyObject*, PyObject*)’:
/u/pez0/fielding/tools/ISCE2_test/build/isce/components/contrib/geo_autoRIFT/autoRIFT/bindings/autoriftcoremodule.cpp:192:50: error: ‘CV_TM_CCOEFF_NORMED’ was not declared in this scope
     cv::matchTemplate( my_imgR, my_imgC, result, CV_TM_CCOEFF_NORMED );
                                                  ^
/u/pez0/fielding/tools/ISCE2_test/build/isce/components/contrib/geo_autoRIFT/autoRIFT/bindings/autoriftcoremodule.cpp: In function ‘PyObject* arPixDisp_s(PyObject*, PyObject*)’:
/u/pez0/fielding/tools/ISCE2_test/build/isce/components/contrib/geo_autoRIFT/autoRIFT/bindings/autoriftcoremodule.cpp:274:50: error: ‘CV_TM_CCORR_NORMED’ was not declared in this scope
     cv::matchTemplate( my_imgR, my_imgC, result, CV_TM_CCORR_NORMED );
                                                  ^
/u/pez0/fielding/tools/ISCE2_test/build/isce/components/contrib/geo_autoRIFT/autoRIFT/bindings/autoriftcoremodule.cpp: In function ‘PyObject* arSubPixDisp_s(PyObject*, PyObject*)’:
/u/pez0/fielding/tools/ISCE2_test/build/isce/components/contrib/geo_autoRIFT/autoRIFT/bindings/autoriftcoremodule.cpp:324:50: error: ‘CV_TM_CCORR_NORMED’ was not declared in this scope
     cv::matchTemplate( my_imgR, my_imgC, result, CV_TM_CCORR_NORMED );
                                                  ^
In file included from /u/vento-r0/ericf/packages/Anaconda/anaconda3/lib/python3.7/site-packages/numpy/core/include/numpy/ndarrayobject.h:21:0,
                 from /u/vento-r0/ericf/packages/Anaconda/anaconda3/lib/python3.7/site-packages/numpy/core/include/numpy/arrayobject.h:4,
                 from /u/pez0/fielding/tools/ISCE2_test/build/isce/components/contrib/geo_autoRIFT/autoRIFT/bindings/autoriftcoremodule.cpp:42:
/u/vento-r0/ericf/packages/Anaconda/anaconda3/lib/python3.7/site-packages/numpy/core/include/numpy/__multiarray_api.h: At global scope:
/u/vento-r0/ericf/packages/Anaconda/anaconda3/lib/python3.7/site-packages/numpy/core/include/numpy/__multiarray_api.h:1463:1: warning: ‘int _import_array()’ defined but not used [-Wunused-function]
 _import_array(void)
 ^
scons: *** [/u/pez0/fielding/tools/ISCE2_test/build/isce/components/contrib/geo_autoRIFT/autoRIFT/bindings/autoriftcoremodule.os] Error 1
scons: done reading SConscript files.
scons: Building targets ...
scons: *** Do not know how to make File target `install' (/u/pez0/fielding/tools/ISCE2_test/isce2/install).  Stop.
scons: building terminated because of errors.

[EJFielding]

As a temporary fix so that I can test my recent changes in PR #171, I commented out #SConscript('geo_autoRIFT/SConscript') in the contrib/SConscript file. Now I can install the rest of ISCE2.

[rtburns-jpl]

With the following change, I'm able to compile using SCons and OpenCV 4.2 on Ubuntu:

diff --git a/contrib/geo_autoRIFT/autoRIFT/bindings/autoriftcoremodule.cpp b/contrib/geo_autoRIFT/autoRIFT/bindings/autoriftcoremodule.cpp
index 94238d9..ccada72 100755
--- a/contrib/geo_autoRIFT/autoRIFT/bindings/autoriftcoremodule.cpp
+++ b/contrib/geo_autoRIFT/autoRIFT/bindings/autoriftcoremodule.cpp
@@ -41,6 +41,7 @@
 #include "iostream"
 #include "numpy/arrayobject.h"
 #include "opencv2/imgproc/imgproc.hpp"
+#include "opencv2/imgproc/types_c.h"
 #include "opencv2/highgui/highgui.hpp"
 #include "opencv2/core/core.hpp"

[piyushrpt]

You can check against CV_MAJOR_VERSION in a preprocessor directive and add the extra header file there - if that is the only change needed.

[rtburns-jpl]

That's what I had at first, but I think always explicitly including types_c is more correct. The header also exists in OpenCV 3.x, so I think the bug was assuming it would always be transitively included.

[leiyangleon]

That's what I had at first, but I think always explicitly including types_c is more correct. The header also exists in OpenCV 3.x, so I think the bug was assuming it would always be transitively included.

@rtburns-jpl @piyushrpt do you suggest to add these two lines (c++11 and c header) and remove the explicit opencv version (v3.4) in the yaml file for the next release of autoRIFT then? Would these be universally applicable to all possible versions of opencv across different platforms?

[rtburns-jpl]

If you haven't already, it might be a good idea to run autoRIFT under cv 4.x, just to make sure there aren't any other regressions in the 3 -> 4 transition that only show up at runtime. Otherwise everything should be fine with those changes

[leiyangleon]

FYI, autoRIFT (v1.0.7) has been upgraded to work with both opencv 3 and 4. So this issue should be resolved.

[rtburns-jpl]

Fixed by https://github.com/isce-framework/isce2/pull/191

[EJFielding]

The new ISCE2 v2.4.1 with the upgraded autoRIFT now compiles with opencv 4 on at least one of the Linux machines that I use (Kamb).

[sgk0]

I have some trouble to get this to work using ISCE2 v2.4.2 on Ubuntu Docker Container (apt install libopencv-dev and python3-opencv). Two things:

1 the #include values in the contrib/geo_autoRIFT are not consistent with how "apt install libopencv-dev python3-opencv" puts it on the ubuntu image (/usr/include/opencv4/opencv2 vs the expected /usr/include/opencv2). But that's easy enough to update by hand.

2 But then the problem percolates downstream to the .hpp files in /usr/include/opencv4/opencv2, because those also appear to have inconsistent #include commands. To me it looks like if opencv2 just installed into /usr/include/opencv2 instead of the above, that would probably make everything work out okay.

So the problem I have doesn't seem to be just related to ISCE, but how opencv installs on different systems? (I did check and everything compiled during the intall without errors, and I was able to import cv2 and cv4 just fine). Maybe adding/usr/include/opencv4 to CPPATH in the SConfigISCE file can fix it?

Are libopencv-dev python3-opencv not compatible, should I use some other ones for apt? In any case, I think it may be worth it to make it optional, or warn persons about this particular issue on ubuntu. Thanks.

[rtburns-jpl]

Adding /usr/include/opencv4 to CPPPATH should be all you need. The opencv4 subdirectory exists on some distros to allow installing multiple versions of opencv side-by-side.

[leiyangleon]

@sgk0 a problem that is similar to yours happened to me as well. When I used MacPorts to install cv4, it installs all the hpp files but no python bindings then I installed the python binding via pip install, which is installed to a different place. In other words, such problems would occur when cv4 and its included hpp files are installed to separate places. Yes, as you speculated, adding the path (where cv4 hpp files are installed) to CPPPATH in SConsfigISCE worked for me.

This is sort of a problem related to the package management system. MacPorts did not upgrade cv4 to including its python variant, pip does not include cv4 hpp files, not sure about apt (seems to have problems). But conda has both the python variant of cv4 and its hpp files. So recommend to use conda install.

[sgk0]

Thank you both, will give that a try. I'm thinking maybe it would be good to update the Configuration control section in the Readme.md file with those kinds of pitfalls as they become apparent. And thanks for the tip on anaconda!

--update: adding /usr/include/opencv4 to the CPPPATH in SConsfigISCE worked for me.