Repackage pynac as a pip-installable package #30534

mkoeppe · 2020-09-09T00:46:09Z

Pynac has a compile-time dependency on the Python library
but is not installed using Python package infrastructure.
This is problematic because Python users cannot install it
using standard Python tools - for example for testing
different Python versions.

It would make sense for src/sage/libs/pynac/pynac.pxd and pynac_wrap.h to be shipped with this Python package, introducing a dependency on Cython. (The only thing blocking this is from sage.libs.gmp.types cimport mpz_t, mpq_t, mpz_ptr, mpq_ptr - but that can be fixed by some minimal cut&paste.)

(The other parts of src/sage/libs/pynac, constant.pxd, constant.pyx, pynac.pyx use various imports from sage and must remain in sage.libs.pynac)

Unfortunately, there is an unrelated project that has claimed the name "Pynac" on pypi.org: https://pypi.org/project/Pynac

Depends on #30446
Depends on #31064

Upstream: Reported upstream. No feedback yet.

CC: @rwst @slel @tobiasdiez @williamstein @kiwifb @tscrim @kliem @videlec @embray

Component: packages: standard

Keywords: pynac, sd110, sd111

Reviewer: François Bissey

Issue created by migration from https://trac.sagemath.org/ticket/30534

The text was updated successfully, but these errors were encountered:

slel · 2020-09-09T15:17:56Z

Changed keywords from none to pynac

slel · 2020-09-09T15:17:56Z

comment:1

Cc-ing Ralf Stephan who maintains Pynac.

slel · 2020-09-14T00:24:24Z

comment:4

Unfortunately, there is an unrelated project
that has claimed the name "Pynac" on pypi.org:

https://pypi.org/project/Pynac

Collision between

Pynac = "Py-Dynac" = Python bindings for the Dynac particle tracking code
Pynac = "Py-GiNaC" = Python derivative of the C++ library GiNaC

Possible solutions:

rename these projects to Py-Dynac and Py-GiNaC, or Pydynac and Pyginac
no renaming but use the name pyginac on PyPI for "Py-GiNaC"

Note:

"Py-Dynac" got the pynac name on PyPI
"Py-GiNaC" got the pynac.org domain name

Regardless of how we handle getting "Py-GiNaC" on PyPI,
a disambiguation note in both "Pynac" projects would be
nice, with links to each other.

I will propose changes via pull requests or other
to add such a note in relevant places.

slel · 2020-09-14T00:28:42Z

comment:5

Or use pynac-ginac as the PyPI name (more discoverable?).

mkoeppe · 2020-10-30T21:15:37Z

comment:7

We briefly discussed it at sd110.

From William Stein (CoCalc) to Everyone: (1:37 PM)
  py-ginac is pretty good.

mkoeppe · 2020-10-30T21:15:37Z

Changed keywords from pynac to pynac, sd110

fchapoton · 2020-11-05T13:53:13Z

comment:9

There is already some py_ginac somewhere :

http://moebinv.sourceforge.net/pyGiNaC.html

mkoeppe · 2020-11-06T23:17:20Z

comment:10

Replying to @fchapoton:

There is already some py_ginac somewhere :

http://moebinv.sourceforge.net/pyGiNaC.html

Thanks for the pointer. This appears to be an active project.
From https://sourceforge.net/projects/pyginac.moebinv.p/:

"This is a refresh and maintained version of the previous stalled project https://sourceforge.net/projects/pyginac/ It works with the current version of GiNaC."

The source (git) is at https://sourceforge.net/p/moebinv/pyginac/code/ci/master/tree/

It provides the Python packages cginac and ginac.

mkoeppe · 2020-11-06T23:26:03Z

comment:11

It calls itself "pyGiNaC" but it does not define a distribution of this name. It must be built using scons and is not pip-installable - so it also does not claim a project name on PyPI.

mkoeppe · 2020-11-10T18:50:06Z

Upstream: Reported upstream. No feedback yet.

mkoeppe · 2020-12-06T17:48:12Z

Changed keywords from pynac, sd110 to pynac, sd110, sd111

mkoeppe · 2021-02-13T20:51:01Z

comment:14

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

mkoeppe · 2021-02-28T03:44:38Z

comment:15

pynac/pynac#371

mkoeppe · 2021-02-28T04:02:17Z

Dependencies: #30446

mkoeppe · 2021-02-28T04:31:30Z

Branch: u/mkoeppe/repackage_pynac_as_a_pip_installable_package

sagetrac-git · 2021-02-28T06:15:10Z

Branch pushed to git repo; I updated commit sha1. New commits:

`44e4dd8`	`Fixup`

sagetrac-git · 2021-02-28T06:15:10Z

Commit: 44e4dd8

mkoeppe · 2021-02-28T07:05:49Z

Author: Matthias Koeppe, ...

mkoeppe · 2021-02-28T07:05:49Z

comment:19

The new package installs correctly. sage.libs.pynac needs more work.

mkoeppe · 2021-02-28T20:37:47Z

comment:21

I think I'll need some help here from people who know about import and cimport in Cython...

tscrim · 2021-02-28T21:43:46Z

comment:22

With what aspects? Just when to use one versus the other?

mkoeppe · 2021-03-02T16:17:15Z

comment:57

Looks like trying to pull in everything to sage.libs.pynac by from ginac.pynac cimport * is a mistake and instead all modules that need pynac should cimport more specifically from ginac.pynac

kliem · 2021-03-02T17:11:01Z

comment:58

Replying to @mkoeppe:

Looks like trying to pull in everything to sage.libs.pynac by from ginac.pynac cimport * is a mistake and instead all modules that need pynac should cimport more specifically from ginac.pynac

But why does

sage: cython(''' 
....: from sage.libs.pynac.pynac cimport * 
....: ''')

works just fine on develop? I would say that things should work just fine, once everything is set up correctly. However, I don't know how to resolve the problems as I'm currently not even able to cimport from my own little two files project.

kliem · 2021-03-02T18:22:22Z

comment:59

I might have found the problem:

In setup.py:

-       package_data = {'ginac': ['*.pxd', '*.h', '*.hpp']},
+       package_data = {'ginac': ['*.pxd', '*.h', '*.hpp', '*.cpp']},

This will place the .cpp files in the correct directory. We need them, because when we cimport we recompile some of them. When the .cpp files are not there, we get the missing symbols.

After doing this, I get a new error:

[sagelib-9.3.beta7] g++ -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-z,relro -Wl,-rpath-link,/srv/public/kliem/sage/local/lib -L/srv/public/kliem/sage/local/lib -Wl,-rpath,/srv/public/kliem/sage/local/lib -march=native -O3 -g -Wdate-time -D_FORTIFY_SOURCE=2 build/temp.linux-x86_64-3.7/build/cythonized/sage/libs/pynac/constant.o -lflint -lgmp -lginac -o build/lib.linux-x86_64-3.7/sage/libs/pynac/constant.cpython-37m-x86_64-linux-gnu.so -L/srv/public/kliem/sage/local/lib -lfactory -lntl -lgmp -lomalloc -lsingular_resources
[sagelib-9.3.beta7] /usr/bin/ld: cannot find -lginac
[sagelib-9.3.beta7] collect2: error: ld returned 1 exit status
[sagelib-9.3.beta7] g++ -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-z,relro -Wl,-rpath-link,/srv/public/kliem/sage/local/lib -L/srv/public/kliem/sage/local/lib -Wl,-rpath,/srv/public/kliem/sage/local/lib -march=native -O3 -g -Wdate-time -D_FORTIFY_SOURCE=2 build/temp.linux-x86_64-3.7/build/cythonized/sage/symbolic/ring.o -lflint -lgmp -lginac -o build/lib.linux-x86_64-3.7/sage/symbolic/ring.cpython-37m-x86_64-linux-gnu.so -L/srv/public/kliem/sage/local/lib -lfactory -lntl -lgmp -lomalloc -lsingular_resources
[sagelib-9.3.beta7] /usr/bin/ld: cannot find -lginac
[sagelib-9.3.beta7] collect2: error: ld returned 1 exit status
[sagelib-9.3.beta7] g++ -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-z,relro -Wl,-rpath-link,/srv/public/kliem/sage/local/lib -L/srv/public/kliem/sage/local/lib -Wl,-rpath,/srv/public/kliem/sage/local/lib -march=native -O3 -g -Wdate-time -D_FORTIFY_SOURCE=2 build/temp.linux-x86_64-3.7/build/cythonized/sage/symbolic/function.o -lflint -lgmp -lginac -o build/lib.linux-x86_64-3.7/sage/symbolic/function.cpython-37m-x86_64-linux-gnu.so -L/srv/public/kliem/sage/local/lib -lfactory -lntl -lgmp -lomalloc -lsingular_resources
[sagelib-9.3.beta7] /usr/bin/ld: cannot find -lginac
[sagelib-9.3.beta7] collect2: error: ld returned 1 exit status
[sagelib-9.3.beta7] g++ -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-z,relro -Wl,-rpath-link,/srv/public/kliem/sage/local/lib -L/srv/public/kliem/sage/local/lib -Wl,-rpath,/srv/public/kliem/sage/local/lib -march=native -O3 -g -Wdate-time -D_FORTIFY_SOURCE=2 build/temp.linux-x86_64-3.7/build/cythonized/sage/libs/pynac/pynac.o -L/srv/public/kliem/sage/local/lib -lgsl -lflint -lgmp -lm -lopenblas -lginac -o build/lib.linux-x86_64-3.7/sage/libs/pynac/pynac.cpython-37m-x86_64-linux-gnu.so -L/srv/public/kliem/sage/local/lib -lfactory -lntl -lgmp -lomalloc -lsingular_resources
[sagelib-9.3.beta7] /usr/bin/ld: cannot find -lginac
[sagelib-9.3.beta7] collect2: error: ld returned 1 exit status
[sagelib-9.3.beta7] g++ -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-z,relro -Wl,-rpath-link,/srv/public/kliem/sage/local/lib -L/srv/public/kliem/sage/local/lib -Wl,-rpath,/srv/public/kliem/sage/local/lib -march=native -O3 -g -Wdate-time -D_FORTIFY_SOURCE=2 build/temp.linux-x86_64-3.7/build/cythonized/sage/symbolic/expression.o -lflint -lgmp -lginac -o build/lib.linux-x86_64-3.7/sage/symbolic/expression.cpython-37m-x86_64-linux-gnu.so -L/srv/public/kliem/sage/local/lib -lfactory -lntl -lgmp -lomalloc -lsingular_resources -lpari
[sagelib-9.3.beta7] /usr/bin/ld: cannot find -lginac
[sagelib-9.3.beta7] collect2: error: ld returned 1 exit status
[sagelib-9.3.beta7] error: command 'g++' failed with exit status 1

But I think, this might be easier to resolve.

mkoeppe · 2021-03-02T18:26:46Z

comment:60

This can't be the right fix. The .cpp files should not be installed.

mkoeppe · 2021-03-02T18:30:47Z

comment:61

Where's that -lginac coming from??

sagetrac-git · 2021-03-02T18:32:28Z

Branch pushed to git repo; I updated commit sha1. New commits:

`b4da02f`	`cimport directly from ginac.pynac`

sagetrac-git · 2021-03-02T18:32:28Z

Changed commit from 93fe11f to b4da02f

kliem · 2021-03-02T18:43:27Z

comment:63

Same problem. Same undefined symbol.

Replying to @sagetrac-git:

Branch pushed to git repo; I updated commit sha1. New commits:

`b4da02f`	`cimport directly from ginac.pynac`

kliem · 2021-03-02T18:46:16Z

comment:64

Replying to @mkoeppe:

This can't be the right fix. The .cpp files should not be installed.

When you cimport you recompile .pxd file. And by this it is trying to recompile everything that was included there. If the corresponding .cpp files aren't found, there is a problem with that.

If we don't want to recompile everything, then we should put the cdef extern from into pynac.pyx and only do the type definitions in pynax.pxd.

mkoeppe · 2021-03-02T18:51:04Z

comment:65

Replying to @kliem:

we should put the cdef extern from into pynac.pyx and only do the type definitions in pynax.pxd.

I don't know about this mechanism, but it sounds like what I am looking for. Could you elaborate on this or push a change to the branch?

kliem · 2021-03-02T18:54:30Z

comment:66

I'll check, if that fixes things.

kliem · 2021-03-03T10:13:59Z

comment:67

I don't know, if that will work. I get many errors and by no means do I know how to fix them.

E.g.

[pynac-0.7.28.p18]   gcc -DNDEBUG -g -fwrapv -O2 -Wall -march=native -O3 -g -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -Iginac -I./ginac -I. -I. -I/srv/public/kliem/sage/local/lib/python3.7/site-packages/pip/_vendor/pep517 -I/srv/public/kliem/sage/local -I/usr/lib/python37.zip -I/usr/lib/python3.7 -I/usr/lib/python3.7/lib-dynload -I/srv/public/kliem/sage/local/lib/python3.7/site-packages -I/srv/public/kliem/sage/local/include -I/usr/include/python3.7m -c ginac/pynac.cpp -o build/temp.linux-x86_64-3.7/ginac/pynac.o -std=c++11 -DSING_NDEBUG -DOM_NDEBUG -DSING_NDEBUG -DOM_NDEBUG -I/srv/public/kliem/sage/local/include
[pynac-0.7.28.p18]   ginac/pynac.cpp:1875:50: error: no declaration matches '__pyx_t_5ginac_5pynac_GFunctionOptVector& GiNaC::function::registered_functions()'
[pynac-0.7.28.p18]    static __pyx_t_5ginac_5pynac_GFunctionOptVector &GiNaC::function::registered_functions(void); /*proto*/
[pynac-0.7.28.p18]                                                     ^~~~~
[pynac-0.7.28.p18]   In file included from ginac/ginac.h:54,
[pynac-0.7.28.p18]                    from ginac/pynac_wrap.h:13,
[pynac-0.7.28.p18]                    from ginac/pynac.cpp:786:
[pynac-0.7.28.p18]   ginac/function.h:409:41: note: candidate is: 'static std::vector<GiNaC::function_options>& GiNaC::function::registered_functions()'
[pynac-0.7.28.p18]     static std::vector<function_options> & registered_functions();
[pynac-0.7.28.p18]                                            ^~~~~~~~~~~~~~~~~~~~
[pynac-0.7.28.p18]   ginac/function.h:340:7: note: 'class GiNaC::function' defined here
[pynac-0.7.28.p18]    class function : public exprseq

I don't even know if it is possible to import a cppclass in the .pyx part and to define in in the header then.

This option is definitely not mentioned here: https://cython.readthedocs.io/en/latest/src/userguide/wrapping_CPlusPlus.html

The problem with this entire setup is that ginac was never properly compiled and linked as a C++ module. I have the feeling that you are trying to achieve something that cython doesn't want you to do.

If you have a C++ library you can create a cython header for this and directly expose the cpp classes via that header (e.g. you can create a pip installable GMP header that uses an existing GMP installation).
If you have some C++ files and you want to expose them directly by a cython wrapper, but the header of this wrapper will recompile your files and thus every module cimporting from this wrapper needs to be able to recompile this as well.
You can create a proper wrapper class of a cpp class. This class can be used by other modules without recompiling the actual cpp class.

This is what the picture looks like to me.

kliem · 2021-07-01T07:52:31Z

comment:68

I digged into a bit. Not that I know a solution now, but at least I understand somehow what is going on.

The question is really what we want. cdef extern from in a .pxd-file assumes that every file cimporting this is linked against the same library or has access to the same sources. I don't think we can change this. So the way this is we must either compile ginac as a library and link against it or ship the sources (the later being really awful, because we have plenty of duplications and it takes forever to compile).

You can use cdef extern in the pyx however and expose the definitions in the header. This seems a very clean way of doing it, but there are tons of obstructions along the way. E.g. the functions must be declared static: https://cython.readthedocs.io/en/latest/src/userguide/external_C_code.html#implementing-functions-in-c

kliem · 2021-07-01T07:57:26Z

comment:69

And apparently with a cppclass it works even in the header? (Different project, but there it seemed to work.)

I'm confused. I will see, if I can find out, what is causing the problem.

mkoeppe · 2021-07-01T16:29:17Z

comment:70

Thanks for looking into this!

mkoeppe · 2021-08-16T17:27:55Z

Changed author from Matthias Koeppe, ... to none

mkoeppe · 2021-08-16T17:27:55Z

comment:72

New approach: #32386 (Merge pynac as src/sage/symbolics/ginac)

kiwifb · 2021-08-16T21:04:03Z

Reviewer: François Bissey

kiwifb · 2021-08-16T21:04:03Z

comment:74

Sure, let's try the other way.

kliem · 2021-08-27T16:09:47Z

comment:76

Removing the branch to avoid confusion in the future.

kliem · 2021-08-27T16:09:47Z

Changed branch from u/mkoeppe/repackage_pynac_as_a_pip_installable_package to none

kliem · 2021-08-27T16:09:47Z

Changed commit from b4da02f to none

mkoeppe added this to the sage-9.3 milestone Sep 9, 2020

mkoeppe added c: packages: standard labels Sep 9, 2020

This comment has been minimized.

Sign in to view

mkoeppe modified the milestones: sage-9.3, sage-9.4 Feb 13, 2021

mkoeppe modified the milestones: sage-9.4, sage-9.5 Jul 19, 2021

mkoeppe removed this from the sage-9.5 milestone Aug 16, 2021

mkoeppe added the s: needs review label Aug 16, 2021

kiwifb added s: positive review and removed s: needs review labels Aug 16, 2021

mkoeppe removed the s: positive review label Aug 26, 2021

mkoeppe closed this as completed Aug 26, 2021

Repackage pynac as a pip-installable package #30534

Repackage pynac as a pip-installable package #30534

Comments

mkoeppe commented Sep 9, 2020

slel commented Sep 9, 2020

This comment has been minimized.

slel commented Sep 9, 2020

This comment has been minimized.

This comment has been minimized.

slel commented Sep 14, 2020

slel commented Sep 14, 2020

mkoeppe commented Oct 30, 2020

mkoeppe commented Oct 30, 2020

fchapoton commented Nov 5, 2020

mkoeppe commented Nov 6, 2020

mkoeppe commented Nov 6, 2020

mkoeppe commented Nov 10, 2020

mkoeppe commented Dec 6, 2020

mkoeppe commented Feb 13, 2021

mkoeppe commented Feb 28, 2021

mkoeppe commented Feb 28, 2021

mkoeppe commented Feb 28, 2021

sagetrac-git mannequin commented Feb 28, 2021

sagetrac-git mannequin commented Feb 28, 2021

mkoeppe commented Feb 28, 2021

mkoeppe commented Feb 28, 2021

mkoeppe commented Feb 28, 2021

tscrim commented Feb 28, 2021

mkoeppe commented Mar 2, 2021

kliem commented Mar 2, 2021

kliem commented Mar 2, 2021

mkoeppe commented Mar 2, 2021

mkoeppe commented Mar 2, 2021

sagetrac-git mannequin commented Mar 2, 2021

sagetrac-git mannequin commented Mar 2, 2021

kliem commented Mar 2, 2021

kliem commented Mar 2, 2021

mkoeppe commented Mar 2, 2021

kliem commented Mar 2, 2021

kliem commented Mar 3, 2021

kliem commented Jul 1, 2021

kliem commented Jul 1, 2021

mkoeppe commented Jul 1, 2021

mkoeppe commented Aug 16, 2021

mkoeppe commented Aug 16, 2021

kiwifb commented Aug 16, 2021

kiwifb commented Aug 16, 2021

kliem commented Aug 27, 2021

kliem commented Aug 27, 2021

kliem commented Aug 27, 2021