Opened 10 years ago

Closed 10 years ago

#10214 closed defect (fixed)

Add numpy dependencies to module_list.py

Reported by: jdemeyer Owned by: GeorgSWeber
Priority: blocker Milestone: sage-4.6.1
Component: build Keywords: numpy upgrade
Cc: leif, maldun Merged in: sage-4.6.1.alpha1
Authors: Jeroen Demeyer Reviewers: Leif Leonhardy
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

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

Attachments (3)

10124_numpy_depends.patch (9.8 KB) - added by jdemeyer 10 years ago.
10124_numpy_depends.2.patch (19.2 KB) - added by jdemeyer 10 years ago.
10214_numpy_depends.patch (19.2 KB) - added by jdemeyer 10 years ago.
Apply ONLY THIS PATCH to sagelib

Download all attachments as: .zip

Change History (18)

Changed 10 years ago by jdemeyer

comment:1 Changed 10 years ago by jdemeyer

  • Cc leif added
  • Description modified (diff)

Changed 10 years ago by jdemeyer

Changed 10 years ago by jdemeyer

Apply ONLY THIS PATCH to sagelib

comment:2 Changed 10 years ago by leif

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)

comment:3 Changed 10 years ago by jdemeyer

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

comment:4 Changed 10 years ago by jdemeyer

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

comment:5 follow-up: Changed 10 years ago by leif

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).

comment:6 in reply to: ↑ 5 Changed 10 years ago by leif

Replying to leif:

(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.

comment:7 follow-up: Changed 10 years ago by leif

# 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.

comment:8 Changed 10 years ago by jdemeyer

  • Status changed from new to needs_review

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.

comment:9 in reply to: ↑ 7 ; follow-up: Changed 10 years ago by jdemeyer

Replying to leif:

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...

comment:10 in reply to: ↑ 9 ; follow-up: Changed 10 years ago by leif

Replying to jdemeyer:

Replying to leif:

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.)

comment:11 Changed 10 years ago by leif

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.

comment:12 in reply to: ↑ 10 Changed 10 years ago by jdemeyer

Replying to leif:

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)

comment:13 follow-up: Changed 10 years ago by leif

  • Cc maldun added
  • Reviewers set to Leif Leonhardy
  • Status changed from needs_review to positive_review

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).

comment:14 in reply to: ↑ 13 Changed 10 years ago by leif

Replying to leif:

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.

comment:15 Changed 10 years ago by jdemeyer

  • Merged in set to sage-4.6.1.alpha1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.