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

Add numpy dependencies to module_list.py #10214

Closed
jdemeyer opened this issue Nov 4, 2010 · 19 comments
Closed

Add numpy dependencies to module_list.py #10214

jdemeyer opened this issue Nov 4, 2010 · 19 comments

Comments

@jdemeyer
Copy link

jdemeyer commented Nov 4, 2010

In order to fix sage -upgrade after #9808, we need to add missing numpy dependencies in module_list.py.

CC: @nexttime @sagetrac-maldun

Component: build

Keywords: numpy upgrade

Author: Jeroen Demeyer

Reviewer: Leif Leonhardy

Merged: sage-4.6.1.alpha1

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

@jdemeyer
Copy link
Author

jdemeyer commented Nov 4, 2010

comment:1

Attachment: 10124_numpy_depends.patch.gz

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

jdemeyer commented Nov 4, 2010

Attachment: 10124_numpy_depends.2.patch.gz

@jdemeyer
Copy link
Author

jdemeyer commented Nov 4, 2010

Apply ONLY THIS PATCH to sagelib

@nexttime
Copy link
Mannequin

nexttime mannequin commented Nov 4, 2010

comment:2

Attachment: 10214_numpy_depends.patch.gz

I counted only 13 extension modules that did not get rebuilt by sage -b:

Updating Cython code....
Building sage/calculus/riemann.pyx because it depends on /home/leif/Sage/sage-4.6.1.alpha0/local/lib/python/site-packages/Cython/Includes/numpy.pxd.
Building sage/calculus/interpolators.pyx because it depends on /home/leif/Sage/sage-4.6.1.alpha0/local/lib/python/site-packages/Cython/Includes/numpy.pxd.
Building sage/finance/time_series.pyx because it depends on /home/leif/Sage/sage-4.6.1.alpha0/local/lib/python/site-packages/Cython/Includes/numpy.pxd.
Building sage/matrix/change_ring.pyx because it depends on /home/leif/Sage/sage-4.6.1.alpha0/local/lib/python/site-packages/Cython/Includes/numpy.pxd.
Building sage/matrix/matrix_complex_double_dense.pyx because it depends on /home/leif/Sage/sage-4.6.1.alpha0/local/lib/python/site-packages/Cython/Includes/numpy.pxd.
Building sage/matrix/matrix_double_dense.pyx because it depends on /home/leif/Sage/sage-4.6.1.alpha0/local/lib/python/site-packages/Cython/Includes/numpy.pxd.
Building sage/matrix/matrix_real_double_dense.pyx because it depends on /home/leif/Sage/sage-4.6.1.alpha0/local/lib/python/site-packages/Cython/Includes/numpy.pxd.
Building sage/modules/vector_complex_double_dense.pyx because it depends on /home/leif/Sage/sage-4.6.1.alpha0/local/lib/python/site-packages/Cython/Includes/numpy.pxd.
Building sage/modules/vector_double_dense.pyx because it depends on /home/leif/Sage/sage-4.6.1.alpha0/local/lib/python/site-packages/Cython/Includes/numpy.pxd.
Building sage/modules/vector_real_double_dense.pyx because it depends on /home/leif/Sage/sage-4.6.1.alpha0/local/lib/python/site-packages/Cython/Includes/numpy.pxd.
Building sage/plot/complex_plot.pyx because it depends on /home/leif/Sage/sage-4.6.1.alpha0/local/lib/python/site-packages/Cython/Includes/numpy.pxd.
Building sage/plot/plot3d/implicit_surface.pyx because it depends on /home/leif/Sage/sage-4.6.1.alpha0/local/lib/python/site-packages/Cython/Includes/numpy.pxd.
Building sage/rings/polynomial/real_roots.pyx because it depends on /home/leif/Sage/sage-4.6.1.alpha0/local/lib/python/site-packages/Cython/Includes/numpy.pxd.
Execute 13 commands (using 8 threads)

@jdemeyer
Copy link
Author

jdemeyer commented Nov 4, 2010

comment:3

What I did was grep all pyx files for numpy and then added the numpy dependency for all those files.

@jdemeyer
Copy link
Author

jdemeyer commented Nov 4, 2010

comment:4

I believe this patch to be finished, but I will do some more testing before putting it officially to review.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Nov 4, 2010

comment:5

Patch looks fine so far; I think it will perhaps trigger more rebuilds than necessary, but won't leave out dependent modules (at first glance).

Defining the other includes and depends also looks much better. (We could create a class for libraries, perhaps in a separate file, or read definitions from [a] text file[s].)

Note that there's also a lot of crap in module_list.py (unused libraries listed, wrong language, ...), and - worse - the Cython files (.pyx, .pxi, .pxd) themselves import or include a lot of unnecessary stuff (cf. e.g. #4000).

@nexttime
Copy link
Mannequin

nexttime mannequin commented Nov 4, 2010

comment:6

Replying to @nexttime:

(We could create a class for libraries, perhaps in a separate file, or read definitions from [a] text file[s].)

... and create our own derived Extension class that automates more, or at least takes some more constructor arguments.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Nov 4, 2010

comment:7
# We pick a file from numpy which is autogenerated so it has the 
# timestamp of the numpy build. 
numpy_depends = [SAGE_LOCAL + '/lib/python/site-packages/numpy/core/include/numpy/_numpyconfig.h']

Using an autogenerated file is of course safe, but we could also handle proper touching (as needed) in NumPy's spkg-install (cf. #4797); bumping the (Sage) patch level usually does not require a rebuild of modules depending on such an spkg.

@jdemeyer
Copy link
Author

jdemeyer commented Nov 5, 2010

comment:8

Upgrade path for testing: http://sage.math.washington.edu/home/jdemeyer/release/sage-4.6.1.alpha0-upgrade/sage-4.6.1.alpha0-upgrade/

I tested an upgrade from 4.6 and it worked fine.

@jdemeyer
Copy link
Author

jdemeyer commented Nov 5, 2010

comment:9

Replying to @nexttime:

Using an autogenerated file is of course safe, but we could also handle proper touching (as needed) in NumPy's spkg-install (cf. #4797); bumping the (Sage) patch level usually does not require a rebuild of modules depending on such an spkg.

You mean touching only if numpy's spkg-install detects that we really are upgrading to a new version of numpy as opposed to upgrading from numpy-1.5.0.spkg to numpy-1.5.0.p0.spkg? That looks like a lot of hassle for a small gain...

@nexttime
Copy link
Mannequin

nexttime mannequin commented Nov 5, 2010

comment:10

Replying to @jdemeyer:

Replying to @nexttime:

Using an autogenerated file is of course safe, but we could also handle proper touching (as needed) in NumPy's spkg-install (cf. #4797); bumping the (Sage) patch level usually does not require a rebuild of modules depending on such an spkg.

You mean touching only if numpy's spkg-install detects that we really are upgrading to a new version of numpy as opposed to upgrading from numpy-1.5.0.spkg to numpy-1.5.0.p0.spkg?

At least that. (In rare cases, a Sage-only update might also require rebuilding the libraries though, which the spkg [maintainer] will be aware of and do the touch, too.)

Many upstream upgrades will not require rebuilding the extension modules at all.

That looks like a lot of hassle for a small gain...

:-) Depends on your hardware...

It's just that there are so many "brute-force" schemes in Sage that could be much cleaner, not necessarily to speed up the build process but because they break potential improvements, frequently cause trouble and make Sage less maintainable.

But I'm ok with your patch. There's always a next release for further improvements... ;-)

(I assume the upgrade path contains your unmodified patch.)

@nexttime
Copy link
Mannequin

nexttime mannequin commented Nov 5, 2010

comment:11

P.S.: In the case of NumPy (#9808), I also used _numpyconfig.h, but apparently forgot to post that on the long ticket I could then no longer follow.

An alternative is to use Cython's NumPy header (local/lib/python/site-packages/Cython/Includes/numpy.pxd, which all relevant extension modules include) and in addition NumPy's C headers this file depends on, which are numpy/arrayobject.h and numpy/ufuncobject.h.

I haven't tested this though; in principle our build system should be smart enough to deduce these indirect dependencies by itself.

@jdemeyer
Copy link
Author

jdemeyer commented Nov 5, 2010

comment:12

Replying to @nexttime:

Many upstream upgrades will not require rebuilding the extension modules at all.

Trying to automagically detect when to rebuild modules depending on numpy looks error-prone to me. Rebuilding them for every numpy upgrade might not be necessary, but I don't think it is that much overkill (at least you have to agree that rebuilding only selected modules is better than a ./sage -ba)

@nexttime
Copy link
Mannequin

nexttime mannequin commented Nov 5, 2010

Reviewer: Leif Leonhardy

@nexttime
Copy link
Mannequin

nexttime mannequin commented Nov 5, 2010

comment:13

So it's not impossible... ;-)

Successfully upgraded from Sage 4.5.3 (built from scratch, without renaming the directory) on Ubuntu 10.04 x86_64.

ptestlong in progress, though I don't expect any issues (at least not caused by NumPy or this patch).

@nexttime
Copy link
Mannequin

nexttime mannequin commented Nov 5, 2010

comment:14

Replying to @nexttime:

Successfully upgraded from Sage 4.5.3 (built from scratch, without renaming the directory) on Ubuntu 10.04 x86_64.

ptestlong in progress, though I don't expect any issues (at least not caused by NumPy or this patch).

As expected, ptestlong passed all tests.

@jdemeyer
Copy link
Author

Merged: sage-4.6.1.alpha1

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

No branches or pull requests

1 participant