kpeeters / cadabra2

A field-theory motivated approach to computer algebra.
https://cadabra.science/
GNU General Public License v3.0
230 stars 37 forks source link

cadabra2 fails to build with Python 3.12 #291

Open doko42 opened 10 months ago

doko42 commented 10 months ago

see https://bugs.debian.org/1061319

we currently have both 3.11 and 3.12 installed during the build, and cadabra2 picks up 3.11, ignores 3.12. is there a way to use a specific Python version?

kpeeters commented 10 months ago

@doko42 This is related to pybind11 which is included in the source tree, and picks up the wrong python. Can you check release 2.4.5.4 (I just tagged & released one on github) and let me know? That one includes a more recent pybind11.

doko42 commented 10 months ago

no, still selects 3.11:


Configuring Python

-- Building for use with Python 3 (good!) -- pybind11 v2.12.0 dev1 CMake Warning (dev) at cmake/modules/FindPythonLibsNew.cmake:60 (find_package): Policy CMP0148 is not set: The FindPythonInterp and FindPythonLibs modules are removed. Run "cmake --help-policy CMP0148" for policy details. Use the cmake_policy command to set the policy and suppress this warning.

Call Stack (most recent call first): libs/pybind11/tools/pybind11Tools.cmake:50 (find_package) libs/pybind11/tools/pybind11Common.cmake:188 (include) libs/pybind11/CMakeLists.txt:211 (include) This warning is for project developers. Use -Wno-dev to suppress it.

-- Found PythonInterp: /usr/bin/python3.11 (found version "3.11.7") -- Found PythonLibs: /usr/lib/x86_64-linux-gnu/libpython3.11.so -- Performing Test HAS_FLTO -- Performing Test HAS_FLTO - Success -- Found python /usr/lib/x86_64-linux-gnu/libpython3.11.so -- Python version is 3.11. -- Installing Python module in /usr/local/lib/python3.11/dist-packages -- Python abi name cpython-311-x86_64-linux-gnu -- Python module extension cpython-311-x86_64-linux-gnu.so

kpeeters commented 10 months ago

Ah, it's picking up a duplicate FindPythonLibsNew.cmake, instead of using the one from pybind11. I have removed it and pushed to github. Can you test against current head or do you need a tagged release?

doko42 commented 10 months ago

that works: https://launchpad.net/ubuntu/+source/cadabra2/2.4.5.4-0ubuntu1

kpeeters commented 10 months ago

Ok. That package still misses a number of dependencies in order to run correctly, in particular texlive. Also, the package that is currently in the repos for 22.04 fails to start because it refers to /usr/lib/python3.10/site-packages/cadabra2 which does not exist (I did not check whether the update for noble has that problem too). Am happy to help to get this to work properly, but I am not really up to date wrt. current debian guidelines as to what should go where, so please let me know how I can help.

alexmyczko commented 7 months ago

@kpeeters sorry for the delay, but i'll try the latest version on debian now... builds but need some more work (tries to use 3.11): http://bananas.debian.net/debian/cadabra2/

a little bit over complicated, basically i have these:

config/Doxyfile:                         core/cadabra2_defaults.py \
client_server/Server.cc:    //  pybind11::eval_file(PYTHON_SITE_PATH"/cadabra2_defaults.py");
client_server/Server.cc:       "f=open(r'" + python_path + "/cadabra2_defaults.py'); "
client_server/Server.cc:       "code=compile(f.read(), 'cadabra2_defaults.py', 'exec'); "
debian/clean:core/cadabra2_defaults.py
jupyterkernel/cadabra2_jupyter/context.py:        with open(os.path.join(SITE_PATH, "cadabra2_defaults.py")) as f:
jupyterkernel/cadabra2_jupyter/context.py:            code = compile(f.read(), "cadabra2_defaults.py", "exec")
doc/modules.dox:/// parts. One is core/cadabra2_defaults.py, which is a pure-python file.
grep: doc/thesis_dominic_price.pdf: binary file matches
core/cadabra2.in:        filepath = os.path.join(d, "cadabra2_defaults.py")
core/cadabra2.in:                code = compile(f.read(), "cadabra2_defaults.py", 'exec')
core/cadabra2.in:        sh.runsource("import os; import sys; install_prefix=os.path.realpath(sys.argv[0]); install_prefix=install_prefix.replace('/bin/cadabra2','${PYTHON_SITE_PATH_REL}'); sys.path.append(install_prefix); print('Cadabra @CADABRA_VERSION_MAJOR@.@CADABRA_VERSION_MINOR@.@CADABRA_VERSION_PATCH@ (build @CADABRA_VERSION_BUILD@ dated @CADABRA_VERSION_DATE@)'); print ('Copyright (C) @COPYRIGHT_YEARS@  Kasper Peeters <kasper.peeters@phi-sci.com>'); f=open('${PYTHON_SITE_PATH}/cadabra2_defaults.py'); code=compile(f.read(), 'cadabra2_defaults.py', 'exec'); exec(code); f.close(); print('Using SymPy version '+sympy.__version__);")
core/cadabra2_defaults.py.in:# \file    cadabra2_defaults.py
core/CMakeLists.txt:   "${PROJECT_SOURCE_DIR}/cadabra2_defaults.py.in"
core/CMakeLists.txt:   "${PROJECT_SOURCE_DIR}/cadabra2_defaults.py"
core/CMakeLists.txt:   # remove_file("${OLD_PYTHON_SITE_PATH}/cadabra2_defaults.py")
core/CMakeLists.txt:   # remove_file("${OLDER_PYTHON_SITE_PATH}/cadabra2_defaults.py")
core/CMakeLists.txt:      "${PROJECT_SOURCE_DIR}/cadabra2_defaults.py" 
core/.gitignore:cadabra2_defaults.py
core/CdbPython.cc:  //      << "   code = compile(f.read(), 'cadabra2_defaults.py', 'exec')\n"
core/CdbPython.cc:// //     << "   code = compile(f.read(), 'cadabra2_defaults.py', 'exec')\n"
core/cadabra2-cli.cc:   // Run cadabra2_defaults.py
core/cadabra2-cli.cc:       execute_file(site_path + "/cadabra2_defaults.py", false);

and i want just one place to set it to /usr/lib/python3/dist-packages/cadabra2_defaults.py maybe better /usr/lib/python3/dist-packages/cadabra2/cadabra2_defaults.py

kpeeters commented 6 months ago

Sorry, github somehow didn't notify me about the edit you made to this comment, so I missed the key part. I'll have a look today. Meanwhile, does the current master branch still insist on building with 3.11 for you?

alexmyczko commented 5 months ago

yes and i want to have a single line configuration for cadabra2_defaults.py path and not 20 random lines distributed across all the files generating it in some magical way. the latest two issues also affect me

kpeeters commented 5 months ago

(Welcome to the mess that is Python package distribution... No matter what I do, there is always some system that decides that they want to do it differently.)

Just to be clear: unless there is a bug, and unless you are inside an AppImage (which doesn't apply to you), then cadabra should only ever try to load cadabra2_defaults.py from PYTHON_SITE_PATH, which you can set at configuration stage using

cmake -DPYTHON_SITE_PATH=...

Is there anything you have found that does not stick to this promise? If so then of course I'm happy to fix that.

(I agree that PYTHON_SITE_PATH/cadabra2/ would have been a better choice, but if I change that now, we will have updates-from-source going haywire on loads of installations. So unless there is a very good reason to change this, I'd prefer not to).

kpeeters commented 1 month ago

The code in 2.5.6 currently on master should be much cleaner about how to pick up Python. @alexmyczko Would you be willing to give this another shot? There are no more references to any absolute path in the binaries, only things relative to the running python module's location.

I did encounter several issues with CMake's FindPython module being not entirely consistent with its own documentation, but generally I think finding Python should now follow https://cmake.org/cmake/help/latest/module/FindPython.html.

alexmyczko commented 1 month ago

I just tried, can it be built with microtex completely disabled?

http://bananas.ethz.ch/debian/cadabra2/4/cadabra2_2.5.6-1_arm64.build

kpeeters commented 1 month ago

I am moving away completely from the latex+dvipng code for typesetting. It is slow (because it runs latex and dvipng on every cell it needs to typeset) and requires a TeX installation which makes it difficult to package cadabra on systems that do not have this naturally (Windows mainly, but also AppImages). So you can no longer disable microtex. If there is something you do not like about microtex, it would probably be best to address that directly, instead of keeping the old typesetting code alive.

kpeeters commented 1 month ago

@alexmyczko So if I understand this correctly, Debian does not like it when the build process fetches additional sources using cmake's FetchContent. While microtex is fetched from a repo which is under my own control and therefore unlikely to go away without cadabra2 itself going away, I understand that Debian prefers that they can just download a single tarball. Would it be sufficient if I released a 'cadabra2_x.y.z_deps_included_for_debian.tar.gz' which has the microtex source included?

alexmyczko commented 1 month ago

yes that would work. unless microtex is used by other software it is not needed be packaged individually

kpeeters commented 1 month ago

The release assets now include a file cadabra2-x.y.z-source-inclusive.tar.gz which includes the relevant microtex branch. Building from this tarball will not fetch any additional files during the build. Can you try this with 2.5.8 which I just released? Thanks!

alexmyczko commented 3 weeks ago

now it builds, but after the installation i get these:

$ cadabra2-cli
FileNotFoundError: Could not open file /usr/local/lib/python3.12/dist-packages/cadabra2_defaults.py
Error encountered while initializing the interpreter

$ cadabra2
Traceback (most recent call last):
  File "/usr/bin/cadabra2", line 114, in <module>
    sh = Shell()
         ^^^^^^^
  File "/usr/bin/cadabra2", line 55, in __init__
    self.convert_data = ConvertData()
                        ^^^^^^^^^^^
NameError: name 'ConvertData' is not defined

i have this:

-DPYTHON_SITE_PATH=/usr/lib/python3/dist-packages/cadabra2
kpeeters commented 3 weeks ago

Those seem to be two unrelated issues. Can you put a build log up somewhere for me to look at?

(it does not help that cmake and pybind11 disagree on the logic to pick up python...).

kpeeters commented 3 weeks ago

Never mind, found it.

kpeeters commented 3 weeks ago

Ok, so the first error (with cadabra2-cli) is caused by the fact that the PYTHON_SITE_PATH which you force by hand is not the same as what

import sysconfig
sysconfig.get_path("platlib")

returns. I know that you are trying to force all cadabra stuff into its own subdir, but that will not work if the above does not agree. I am not sure whether there is any flexibility on this in the Debian guidelines; does it really have to go into lib/python3/dist-packages/cadabra2?

The 2nd one is weird: the ConvertData object is a pybind11-exported object in the C++ code of the cadabra2 python library, but indeed cadabra2.in does not import that, so it should have led to trouble on earlier systems. Probably happening because python3.12 does this lookup before it has executed the code in cadabra2_defaults.py, whereas earlier versions did that lookup later. If you add

import ConvertData from cadabra2

somewhere near the top of core/cadabra2.in this should work.

Let me know about the Debian folder guidelines and I'll roll another tarball for you to try.

kpeeters commented 3 weeks ago

One further issue related to folder structure: the main python library is a C++ shared library cadabra2.so (or rather a version of that with architecture info in it), which I would really like to have in /usr/lib/python3/dist-packages/cadabra2.so so it can imported with import cadabra2. But if we then also have a folder /usr/lib/python3/dist-packages/cadabra2/ I am sure this will confuse at least some of the systems on which cadabra runs currently. Then again, maybe my understanding of how this is supposed to work is wrong.