opencog / atomspace

The OpenCog (hyper-)graph database and graph rewriting system
https://wiki.opencog.org/w/AtomSpace
Other
795 stars 225 forks source link

Broken python bindings #3041

Closed gl-yziquel closed 2 months ago

gl-yziquel commented 9 months ago

Hi.

I've been dabbling with the atomspace in guile, and, because of a need of integration with other personal components in python, I tried to use the python bindings to the atomspace. Which leads me to the observations in this ticket:

  1. The python bindings are not detected out of the box by cmake. I have to pass the -DHAVE_CYTHON=ON option to cmake explicitly. I believe I should not have to.

  2. In the opencog/cython/opencog/CMakeLists.txt, lines 14-16, the INSTALL statement need the -DPYTHON_DEST option to be explicitly set. 2.1. it should be automatically detected 2.2. the cmake process should fail with a more obvious error when PYTHON_DEST is not set 2.3. i still do not know what value I should explicitly pass to this variable.

  3. Be that as it may, when invoking cmakewith explicit values -DHAVE_CYTHON=ONand -DYPTHON_DEST=/somewhere/over/the/rainbow, the build fails with C++ level errors:

/home/dorothy/home/cellar/atomspace/opencog/atoms/grounded/PythonRunner.cc:59:23: error: invalid use of incomplete type ‘class opencog::PythonEval’
   59 |         return applier->apply_v(as, _fname, asargs);
      |                       ^~
In file included from /home/dorothy/home/cellar/atomspace/opencog/atoms/grounded/PythonRunner.cc:28:
/home/dorothy/home/cellar/atomspace/opencog/atoms/grounded/DLPython.h:29:7: note: forward declaration of ‘class opencog::PythonEval’
   29 | class PythonEval;
      |       ^~~~~~~~~~
/home/dorothy/home/cellar/atomspace/opencog/atoms/grounded/PythonRunner.cc: In member function ‘virtual opencog::ValuePtr opencog::PythonRunner::evaluate(opencog::AtomSpace*, const opencog::Handle&, bool)’:
/home/dorothy/home/cellar/atomspace/opencog/atoms/grounded/PythonRunner.cc:70:35: error: invalid use of incomplete type ‘class opencog::PythonEval’
   70 |         return CastToValue(applier->apply_tv(as, _fname, asargs));
      |                                   ^~
In file included from /home/dorothy/home/cellar/atomspace/opencog/atoms/grounded/PythonRunner.cc:28:
/home/dorothy/home/cellar/atomspace/opencog/atoms/grounded/DLPython.h:29:7: note: forward declaration of ‘class opencog::PythonEval’
   29 | class PythonEval;
      |       ^~~~~~~~~~
make[2]: *** [opencog/atoms/grounded/CMakeFiles/grounded.dir/build.make:174: opencog/atoms/grounded/CMakeFiles/grounded.dir/PythonRunner.cc.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:2641: opencog/atoms/grounded/CMakeFiles/grounded.dir/all] Error 2
make: *** [Makefile:156: all] Error 2

Python bindings to the atomspace seem to be broken.

linas commented 9 months ago

I don't understand the bug report. HAVE_CYTHON is set, if cython3 is detected. when you say cmake, it prints a log reporting what it finds. For me, it found:

-- Python 3.11.2 interpreter found.
-- Python 3.11.2 libraries found.
-- Cython ( 0.29.3 >= 0.24.0) found.
-- Python destination dir found: /usr/local/lib/python3.11/dist-packages
-- Python install dir: /usr/local/lib/python3.11/dist-packages/opencog
-- Using nosetests executable /usr/bin/nosetests3

The following components will be built:
-----------------------------------------------
   Python bindings       - Python (cython) bindings.
   Python tests          - Python bindings nose tests.
linas commented 9 months ago

Hmm, opencog/cython/opencog/CMakeLists.txt, lines 14-16, looks fine. It should be a semi-colon-sparated list of directories. the -D doesn't make sense. See

https://cmake.org/cmake/help/latest/variable/CMAKE_INSTALL_RPATH.html https://cmake.org/cmake/help/latest/prop_tgt/INSTALL_RPATH.html

At any rate, it looks like its sort of working, anyway. The runpath is set. You can check with objdump -x libPythonSCM.so |grep RUNPATH

linas commented 9 months ago

The third block of errors appear to happen because -DHAVE_CYTHON was never set in the compiler, and thus, there is no definition for PythonEval. By force-setting it in CMake, you've tricked it into a confusing situation. I will add an error message.

linas commented 9 months ago

I take back the earlier comment. opencog/cython/opencog/CMakeLists.txt, lines 14-16, looks fine. To debug, make this change:

--- a/opencog/cython/opencog/CMakeLists.txt
+++ b/opencog/cython/opencog/CMakeLists.txt
@@ -15,6 +15,8 @@ SET(CMAKE_INSTALL_RPATH
    "${CMAKE_INSTALL_PREFIX}/lib/opencog"
    "${PYTHON_DEST}")

+MESSAGE(STATUS "This is the RUNPATH for cython: ${CMAKE_INSTALL_RPATH}")
+
 INCLUDE_DIRECTORIES(
        ${PYTHON_INCLUDE_DIRS}
        ${CMAKE_CURRENT_SOURCE_DIR}

When I do this, running cmake prints, for me,

-- Setting python RPATH to /usr/local/lib/python3.11/dist-packages/opencog
-- This is the RUNPATH for cython: /usr/local/lib/opencog;/usr/local/lib/python3.11/dist-packages/opencog

I'm guessing its borken for you because either python3 or cython3 aren't installed. Note that python version 2 is not supported in opencog.

gl-yziquel commented 9 months ago

Python3 and Cython3 are installed. I noticed they were not found. So I tried to force it with -DHAVE_CYTHON.

Will try to come back to this as soon as the need for python bindings is being felt again.

linas commented 9 months ago

The search is done by /usr/local/share/opencog/cmake/OpenCogFindPython.cmake which uses /usr/local/share/opencog/cmake/FindPython3Interp.cmake to do lower-level work.

linas commented 9 months ago

To debug, just edit the files by hand, and use

MESSAGE(STATUS "what happened of foo is ${FOO_BAR}")

to print out the variable FOO_BAR. This only prints when cmake is run. The original versions of these files are installed by cogutil, from here: https://github.com/opencog/cogutil/tree/master/cmake

linas commented 9 months ago

My best guess is that you have both python3 and python2 installed. Opencog searches for both, and might have gotten confused during the search. To avoid this confusion, I've removed the search for python2 here: https://github.com/opencog/cogutil/pull/273

gl-yziquel commented 9 months ago

Thank you. I'll attempt that as soon as I can.

linas commented 2 months ago

Closing, I'm guessing this is resolved.