Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merge cypari and cypari2 #19

Open
1 of 2 tasks
videlec opened this issue Jul 20, 2017 · 40 comments
Open
1 of 2 tasks

merge cypari and cypari2 #19

videlec opened this issue Jul 20, 2017 · 40 comments

Comments

@videlec
Copy link
Collaborator

videlec commented Jul 20, 2017

The original cypari written by N. Dunfield and M. Culler will be merged with this project to ease maintenance. Though we are facing several incompatibilities

1 - Windows

cypari can be installed on Windows (however not directly from source, they provide wheels on PyPI). This is at the cost of a fork of cysignals and many special casing.

  • cysignals is not working on windows. See issue 63.
  • issues with long (cypari defines a new pari_ulongword type)

Note that these issues refer to plain Windows. Under cygwin, there are no such issues.

2 - Static vs dynamic linking / source vs distribution

cypari currently ships gmp, pari and (a fork of) cysignals. cypari2 contains only sources for a Python module. The original cypari has the advantage of being installable on many platforms using wheels (among other it solves the problem of having cysignals built after libpari).

We propose in the merged version to have three possible ways of installation

  1. pip install cypari: installs a wheel with static linking and embedded cysignals (i.e. what is currently available with the cypari wheels)

  2. pip install --no-binary cypari: installs from an sdist with dynamic linking from gmp/pari on the system and dependency from cysignals (i.e. the cypari2 version)

  3. (optional) pip install --no-binary --install-option=static-link cypari: installs from an sdist but also builds libpari.a and libgmp.a and statically links against them (intermediate solution)

4 - Cython macro

Cython macros (either defined using DEF or via compile_time_env option of cythonize) modifies the C code generated by Cython from the pyx files. Having many of them imposes to provide a different versions of the generated C files for each possible values of the constants (in order to allow to build from source without Cython).

The Python2/Python3 compatibility can be achieved without any of them (as done in cypari2, see #13). However it seems unavoidable for the Windows port.

@jdemeyer
Copy link
Collaborator

Building gmp and pari would better not be part of the setup.py script.

In my view, this is mostly an problem of user interface. It's trivial to support different kinds of builds. But the hard question is: how does the interface work? What should happen when somebody does pip install cypari? When installing from source using setup.py, it would be possible to require an argument like setup.py --build-pari or whatever. But with wheels, I don't think this is possible.

@videlec
Copy link
Collaborator Author

videlec commented Jul 20, 2017

Building gmp and pari would better not be part of the setup.py script.

In my view, this is mostly an problem of user interface. It's trivial to support different kinds of builds. But the hard question is: how does the interface work? What should happen when somebody does pip install cypari? When installing from source using setup.py, it would be possible to require an argument like setup.py --build-pari or whatever. But with wheels, I don't think this is possible.

This is why I proposed in the exchange of e-mails to have two packages cypari-mini (only source?) and cypari-full (source and wheels). I don't think there is a need for more options. Don't you like this way?

@videlec videlec changed the title merge cypari merge cypari and cypari2 Jul 20, 2017
@NathanDunfield
Copy link

@videlec With regards to cypari-mini versus cypari-full is the idea that they would both install a package named cypari but they would be separate packages on PyPI? Or would they install differently named packages? Either way, we probably need to think about what should happen when pip tries to install another Python package that depends on cypari-*.

@videlec
Copy link
Collaborator Author

videlec commented Jul 20, 2017

With regards to cypari-mini versus cypari-full is the idea that they would both install a package named cypari but they would be separate packages on PyPI? Or would they install differently named packages? Either way, we probably need to think about what should happen when pip tries to install another Python package that depends on cypari-*.

@NathanDunfield my idea is not completely clear yet :-) I definitely want both versions to install a cypari package. The best I think will be to have cypari-full as a "virtual package" that do the install gmp, pari, cysignals, cypari-mini in that order. If this is the way to go, we would perhaps better use the name cypari for cypari-mini because it would match the Python module names. However, this change would break backward compatibility of pip install cypari.

Concerning dependencies on cypari, is it possible to have fine dependencies in pip? I imagine something like: "if cypari-mini is there then fine, otherwise install cypari-full".

@culler
Copy link

culler commented Jul 20, 2017

@videlec @NathanDunfield

A simple implementation of this idea might be to just have a setup.py option which specifies whether to use system libraries or to build the libraries from scratch. The default would be to use system libraries. That way:
pip install --no-binary cypari
would be the equivalent of installing cypari-mini (provided the build succeeds). On the other hand
pip install cypari
would download the wheel, which would necessarily include its own copies of the libraries, so that would be the equivalent of installing cypari-full. To build the wheels one would supply the option to setup.py which indicates to build the libraries.
Someone who wanted to build their own standalone module could just clone the repo.

@NathanDunfield
Copy link

@videlec wrote:

Concerning dependencies on cypari, is it possible to have fine dependencies in pip? I imagine something like: "if cypari-mini is there then fine, otherwise install cypari-full".

You can achieve this sort of effect by putting some logic in the dependent package's setup.py script and modifying the list of packages passed to setup(install_requires=...). We do this with SnapPy, e.g. not including cypari as a requirement when installing into Sage. But so far I haven't seen any built in support for this in pip, e.g. requirements of the form foo>2.0|bar>1.5 which is satisfied if either is present and otherwise it tries to install the first one.

@culler wrote:

On the other hand pip install cypari would download the wheel, which would necessarily include its own copies of the libraries, so that would be the equivalent of installing cypari-full.

One possible issue: support for Linux wheels in pip is pretty new, and so on some distros pip install cypari would actually try to build the source tarball (unless one also posts eggs).

@NathanDunfield
Copy link

@videlec @culler
Another approach would be to have three packages: cypari, cypari-full and cypari-mini. The first contains only a few lines of code that try to import cypari-mini and cypari-full in turn; when it succeeds it sets cypari.pari to cypari_found.pari etc and otherwise raises an ImportError. If you do pip install cypari, then we make this succeed only if one of cypari-full and cypari-mini is present, otherwise it throws polite error message indicating that the user should first install their choice of cypari-full and cypari-mini. We could put in certain shortcuts, e.g. on Windows pip install cypari would automatically require cypari-full. Possibly the same thing should be true on macOS for a Frameworks build of Python (such as the ones downloaded from Python.org) as opposed the "plain unix style" Pythons that come with SageMath and (I think) brew.

For example, one concrete possibility would be to have the setup.py script of cypari do the following:

  1. Try to import cypari-mini and cypari-full. If one is available and has the same version number as cypari then everything is good and it just installs its own few lines of code.

  2. If no suitable version of cypari-* is available and the OS is either Windows or macOS with a Frameworks build of Python, then add cypari-full to install_requirements and pip will handle the rest.

  3. On Linux or with a "plain unix" macOS python, then it could do some tests to see if there appears to be suitable versions of gmp and pari available. If so, add cypari-mini (which forces cysignals) to the install_requirements and otherwise add cypari-full.

@videlec
Copy link
Collaborator Author

videlec commented Jul 21, 2017

@NathanDunfield

Another approach would be to have three packages: cypari, cypari-full and cypari-mini.
[ ... snip ...]

With your proposal, if I want the minimal version should I do

$ pip install cypari-mini
$ pip install cypari
  1. On Linux or with a "plain unix" macOS python, then it could do some tests to see if there appears to be suitable versions of gmp and pari available. If so, add cypari-mini (which forces cysignals) to the install_requirements and otherwise add cypari-full.

It is hard to provide reliable tests to see whether one can build cypari-mini. Namely, the user might not have gcc. I would be more convinced by a solution that by default always install cypari-full unless explicitely specified (as Marc proposed with --no-binary).

@NathanDunfield
Copy link

@videlec wrote:

With your proposal, if I want the minimal version should I do

$ pip install cypari-mini
$ pip install cypari

Yes, that's right.

It is hard to provide reliable tests to see whether one can build cypari-mini. Namely, the user might not have gcc. I would be more convinced by a solution that by default always install cypari-full unless explicitly specified (as Marc proposed with --no-binary).

I agree that figuring out whether cypari-mini can be installed is tricky and should be avoided if possible. I would be happy with a solution that installs cypari-full unless explicitly specified (via --no-binary or some other flag), but wasn't sure if that would be acceptable to you guys.

@NathanDunfield
Copy link

Also, I was thinking that perhaps we should use slightly more descriptive names, perhaps cypari-selfcontained and cypari-system-pari or cypari-static and cypari-libpari.

@videlec
Copy link
Collaborator Author

videlec commented Jul 21, 2017

Then, we might want to stick to a unique cypari package when in source mode (i.e. --no-binary pip option) will use whatever pari is available on the system (that is the current cypari2 version) and for which all the wheels are self-contained (ie the current cypari wheels)? When used in source mode it is assumed that libpari is available and that can be tested for providing nice error message.

I am not sure there is a need for intermediate installations. This is the job of distributions.

I see one backward compatibility with this proposal for people who used to install the original cypari in source mode.

@NathanDunfield
Copy link

So pip has an --install-option flag which can be used to pass options down to setup.py. It seems like this would allow us to cover the backward compatibility case you mention via something like:

pip install --no-binary --install-option=build-own-pari  cypari

Putting this together, a concrete plan would be:

  1. Have a single cypari package on PyPI with the the usual 16 or so cypari-full binary wheels and a source tarball containing the Cythonized C source files but not the gmp and pari sources.

  2. Have the source tarball default to cypari-mini unless a suitable flag is set, in which case it fetches the gmp and pari sources and builds its own copy of pari from scratch.

What do people think of such a plan? It seems good to me.

@culler
Copy link

culler commented Jul 21, 2017

I see one backward compatibility with this proposal for people who used to install the original cypari in source mode.

I think Nathan, Matthias and I will be able to handle it. :^)
Oops, I forgot Bill Allombert! He will be OK too.

@culler
Copy link

culler commented Jul 21, 2017 via email

@jdemeyer
Copy link
Collaborator

What do people think of such a plan? It seems good to me.

I thought a bit more this. One important question is: how to link against libpari? Are we doing static linking (like the current cypari) or dynamic linking?

The problem is that both have significant disadvantages:

  • a static library cannot be shared across modules. So, if there are other packages which link against PARI, they will use a different independent copy of PARI. In particular, cysignals should not be built with PARI support in this case (I am assuming that cysignals will still be a separate package, contrary to what cypari does)
  • a dynamic library seems fundamentally incompatible with wheel. You could ship the dynamic library (the .so file) inside the wheel, but the directory where the library gets installed might not be in the library search path. For system installs, this might be OK but for a --user install, this will almost certainly break.

@jdemeyer
Copy link
Collaborator

I guess what I would prefer is to give the user many options, but make nothing the default. In other words, allow pip install cypari-static-libpari but let pip install cypari fail with an error message explaining the various options.

@isuruf
Copy link
Member

isuruf commented Jul 28, 2017

You can have dynamic libraries inside a wheel. That's what numpy, scipy etc does with openblas.

@culler
Copy link

culler commented Jul 28, 2017 via email

@culler
Copy link

culler commented Jul 28, 2017 via email

@isuruf
Copy link
Member

isuruf commented Jul 28, 2017

Does that work on Windows as well?

Yes.

@jdemeyer
Copy link
Collaborator

You can have dynamic libraries inside a wheel. That's what numpy, scipy etc does with openblas.

I just checked what numpy does. It has an openblas library hidden inside site-packages/numpy/.libs. So it's really meant to be used only by numpy. As far as external packages are concerned, it still has the disadvantages of a static library.

@NathanDunfield
Copy link

NathanDunfield commented Jul 28, 2017

@isuruf: Do other python packages use the version of the openblas library that ships with numpy, or does only the numpy module itself use it? If just the latter, then effectively this is no different than an entirely static wheel.

Just to clarify, the second option @culler refers to means:

pip install --no-binary cypari
(installs from an sdist with dynamic linking against a previous installed system libpari)

@culler
Copy link

culler commented Jul 28, 2017 via email

@jdemeyer
Copy link
Collaborator

The most important one is that it allows more flexibility with the configuration of modules.

You are right that it fixes this problem, but that's also the only problem that the numpy approach fixes.

@culler
Copy link

culler commented Jul 28, 2017 via email

@isuruf
Copy link
Member

isuruf commented Jul 28, 2017

@isuruf: Do other python packages use the version of the openblas library that ships with numpy, or does only the numpy module itself use it? If just the latter, then effectively this is no different than an entirely static wheel.

Both scipy and numpy have openblas dependency. And both ship the exact same version of openblas shared library. I'm guessing that whichever package is imported first in python brings in openblas and only that copy is loaded.

@jdemeyer
Copy link
Collaborator

And both ship the exact same version of openblas shared library. I'm guessing that whichever package is imported first in python brings in openblas and only that copy is loaded.

That seems to be the case on Linux at least. I have no idea what happens on other systems (given that dynamic linking is very system-dependent, you cannot really extrapolate from Linux).

Suppose that this really works on all systems where we want to build wheels for. Then we could make a Python package libpari which just contains the PARI dynamic library and then use it from cypari and cysignals using the same approach as numpy and scipy.

@jdemeyer
Copy link
Collaborator

One way to check: install numpy and scipy from the binary wheel and then delete the directory .../site-packages/scipy/.libs/ If you can still import scipy.stats.statlib, then the trick works.

@isuruf
Copy link
Member

isuruf commented Jul 28, 2017

One way to check: install numpy and scipy from the binary wheel and then delete the directory .../site-packages/scipy/.libs/ If you can still import scipy.stats.statlib, then the trick work

Yes, it works. Removing numpy/.libs and keeping scipy/.libs doesn't work because scipy imports numpy first.

@NathanDunfield
Copy link

One way to check: install numpy and scipy from the binary wheel and then delete the directory .../site-packages/scipy/.libs/ If you can still import scipy.stats.statlib, then the trick works.

I took a stab at this. On macOS, the analogous directory appears to be .../site-packages/scipy/.dylibs/ which contains:

libgcc_s.1.dylib     libgfortran.3.dylib* libquadmath.0.dylib* 

However, numpy has no .dylibs directory, and numpy/core/multiarray.cpython-36m-darwin.so is only linked against:

	/System/Library/Frameworks/Accelerate.framework/Versions/A/Accelerate (compatibility version 1.0.0, current version 4.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1213.0.0)

My guess is the former includes Apple's version of blas. For funsies, I did try deleting scipy/.dylibs/ but import scipy.stats.statlib failed as you might expect.

On Windows, there is no wheel for scipy at all, so I wasn't able to get data there at all.

@jdemeyer
Copy link
Collaborator

Yes, it works.

On which system?

@isuruf
Copy link
Member

isuruf commented Jul 28, 2017

On which system?

Linux

@NathanDunfield
Copy link

I guess what I would prefer is to give the user many options, but make nothing the default. In other words, allow pip install cypari-static-libpari but let pip install cypari fail with an error message explaining the various options.

Unfortunately, such an approach would cause pip install snappy to throw an error message if cypari is not already installed. For that reason, I favor the "install statically unless explicitly told otherwise" strategy that Marc outlined above.

I believe this distinction really only matters on Linux in the context of individual users installing cypari into either their own or the system's Python. On Windows, static is currently the only option and it is almost always the best choice on macOS (in part since there is no standard location for libpari). Distributions, including SageMath, can of course handle any reasonable approach we take regardless of platform.

For an individual Linux user, perhaps doing pip install cypari --user, I don't see any major downsides to defaulting to the static wheel. Sure, they might end up with a second copy of libpari on their system, but it should be hidden from anything else. And on the plus side, the install is much more likely to succeed since who knows what version of libpari they have installed...

@jdemeyer
Copy link
Collaborator

jdemeyer commented Mar 4, 2019

I asked distutils-sig for the correct way to deal with dynamic libraries and wheels and the answer was basically that it's not supported.

So as long as Python doesn't fix this, cypari2 will never be packagable as wheel.

For cypari, you avoid this problem by compiling everything (including the cysignals fork) in a single Python module.

@tobiasdiez
Copy link

What's the current status here? It would be very handy to have pre-build wheels and Windows support.

@NathanDunfield
Copy link

@tobiasdiez Unfortunately, no progress has been made on this, though I think the two versions are still in reasonably close sync (both on Pari 2.11.3). CyPari is now also hosted here on GitHub and we continue to provide pre-built wheels for Windows, Linux, and Macs on PyPI that include Pari itself as a static library.

If pip was a more sophisticated package manager a merger would be easier. We basically need a meta-package "cypari" that is satisfied by either cypari-static (that is, the currentcypari) or cypari-dynamic (that is, the current cypari2). As it is, I don't see how to accomplish this cleanly --- I'd guess it would be straightforward to have a single repo and some scripts for exporting both versions, but it's not clear what this accomplishes beyond allowing one to close this ticket. ;-)

@mkoeppe
Copy link

mkoeppe commented Mar 19, 2021

We basically need a meta-package "cypari" that is satisfied by either cypari-static (that is, the currentcypari) or cypari-dynamic (that is, the current cypari2). As it is, I don't see how to accomplish this cleanly --- I'd guess it would be straightforward to have a single repo and some scripts for exporting both versions, but it's not clear what this accomplishes beyond allowing one to close this ticket. ;-)

I think this could be resolved in a very simple way. The unified cypari package (when installed from source) should just do what the current cypari2 package does - use an existing installation of PARI; a wheel built from it using setup.py bdist_wheel would obviously not be distributable but can be installed by the user in multiple venvs.

For building distributable wheels (with the included copy of PARI) to be put on PyPI, like the current cypari package, one would use a special build script, or it would be activated only by a special option to setup.py bdist_wheel or an environment variable.

Users can then choose between installing the "static" and "dynamic" version by just using pip install options such as --no-binary, --only-binary, --prefer-binary.

@NathanDunfield
Copy link

@mkoeppe That's a promising idea. One potential issue is that the current cypari and cypari2 packages have different dependencies: only cypari2 depends on cysignals whereas cypari contains an embedded copy of it (which is necessary on Windows). However, my belief is that the list of dependencies is embedded in each tarball/wheel and not something that PyPI associates to a package release directly, so this shouldn't be a problem. Indeed, snappy's Windows wheels do have an extra dependency (pyreadline) that the others don't and this has worked fine.

@mkoeppe
Copy link

mkoeppe commented Mar 21, 2021

my belief is that the list of dependencies is embedded in each tarball/wheel and not something that PyPI associates to a package release directly, so this shouldn't be a problem.

I agree.

@videlec
Copy link
Collaborator Author

videlec commented Dec 15, 2021

We basically need a meta-package "cypari" that is satisfied by either cypari-static (that is, the currentcypari) or cypari-dynamic (that is, the current cypari2). As it is, I don't see how to accomplish this cleanly --- I'd guess it would be straightforward to have a single repo and some scripts for exporting both versions, but it's not clear what this accomplishes beyond allowing one to close this ticket. ;-)

I think this could be resolved in a very simple way. The unified cypari package (when installed from source) should just do what the current cypari2 package does - use an existing installation of PARI; a wheel built from it using setup.py bdist_wheel would obviously not be distributable but can be installed by the user in multiple venvs.

For building distributable wheels (with the included copy of PARI) to be put on PyPI, like the current cypari package, one would use a special build script, or it would be activated only by a special option to setup.py bdist_wheel or an environment variable.

Users can then choose between installing the "static" and "dynamic" version by just using pip install options such as --no-binary, --only-binary, --prefer-binary.

This sounds promising. However, I am worried that a "bundled" install might actually deserve the project. Here are two problematic points that I can see

  • It would be complicated to use cypari inside Cython code because the compiler has no idea where to find the PARI header files and the linker has no idea where to find the PARI lib. This might be possible to patch by including the proper compiler directives inside the pxd files though the installation path is unknown at the time the wheel is built
  • If "bundled" cypari and "bundled" gmpy2 are both installed, then each of them would be linked to their own libgmp (with possibly different versions). That likely prevents the usage of Cython code using both cypari and gmpy2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants