Opened 15 years ago

Closed 14 years ago

#1261 closed enhancement (fixed)

[With patch, positive review] parallel "sage -br"

Reported by: William Stein Owned by: Gary Furnish
Priority: major Milestone: sage-3.0.1
Component: packages: standard Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

It would be very desirable for "sage -br" to take advantage of multi-processor and multi-core machines automatically.

The following irc session describes an idea for a way to easily implement this. The main idea is to use the dry_run option to setup(...) in setup.py, capture the output of the dry_run, then run those commands in parallel.

[9:21pm] was_: When "sage -br" runs we first (with custom code I wrote) run cython on a bunch of files -- for this the order is probably important.
[9:21pm] was_: (??)
[9:22pm] was_: Anyway, after that, distutils runs gcc or g++ on a large number of files.  I think for that the order doesn't matter much.
[9:22pm] cwitty: The order of running Cython should not be important.
[9:22pm] was_: Are you sure?
[9:22pm] mabshoff: ok
[9:22pm] was_: There is a dry_run option to the setup function in devel/sage/setup.py.
[9:22pm] cwitty: (Cython depends on source files, like .pxd; but I'm pretty sure it doesn't depend on the generated .c or .cpp files.)
[9:23pm] was_: cwitty -- I think you're right.
[9:23pm] mabshoff: each file cython compiles is a dynmaic library?
[9:23pm] was_: Definitely things are easier if you are.
[9:23pm] cwitty: mabshoff, yes.
[9:23pm] was_: cython just makes .c/.cpp/.h files as output; that's it.
[9:23pm] was_: And everything is dynamic.
[9:23pm] mabshoff: Then it shouldn't have compile time dependencies.
[9:23pm] was_: Anyway, back to my idea.
[9:23pm] mabshoff: link time at import, but that doesn't matter.
[9:23pm] was_: (There are possibly some mild dependencies since multiple files are combined to make one dynamic library in some cases.)
[9:24pm] was_: Anyway, back to my idea.
[9:24pm] was_: If you look at devel/sage/setup.py, and put dry_run=True as the last option to the huge setup(...) function,
[9:24pm] was_: then distutils will simply print out all the gcc commands it *would* run.
[9:24pm] was_: But it doesn't run thme.
[9:24pm] mabshoff: I see where you are going 
[9:25pm] was_: So my crazy idea is to run setup once as a dry_run, capture all the output, divide it up, do the gcc's in parallel, then run distutils again with dry_run=False,
[9:25pm] was_: which will be fast.
[9:25pm] mabshoff:
[9:25pm] mabshoff: Let's hope it works.
[9:25pm] was_: I have no idea if it would really work.
[9:25pm] mabshoff: Even then we could also create a makefile from cython info and run parallel make on that.
[9:26pm] was_: If we wanted, though it seems hardly necessary.
[9:26pm] mabshoff: KISS
[9:26pm] was_: yep.
[9:26pm] was_: Make is pretty KISS though, at least the way we're thinking of using it.
[9:26pm] was_: KISS was why the spkg build system uses make.
[9:27pm] mabshoff:
[9:27pm] mabshoff: I just got a timeout with -long running doctests.
[9:27pm] mabshoff: Maybe we should disable TIMEOUT with -long
[9:27pm] was_: There should be a timeout but it should be way longer.
[9:27pm] mabshoff: ok
[9:27pm] was_: E.g., David Joyner put an annoying #long doctest in once that took *days* to run.
[9:27pm] was_: Very annoying.
[9:27pm] was_: It was good that there was a timeout.
[9:28pm] was_: I asked him about it and he said it was a "record breaking" computation...
[9:28pm] mabshoff:
[9:28pm] mabshoff: I think the doctest finishes, but then python realises that the TIMEOUT was exceeded.
[9:29pm] was_: that sounds dumb.
[9:29pm] mabshoff:
[9:29pm] mabshoff: I set my timeout to 30 minutes for now on my copy.
[9:29pm] was_: I just did some tests with my parallel "sage -br" above and it looks very promising.
[9:30pm] mabshoff: Is it going in 2.8.14? *ducks*
[9:30pm] was_: no
[9:30pm] was_: I want to release 2.8.14 in an hour.
[9:30pm] mabshoff: I thought so.
[9:33pm] was_: For smaller files running cython takes longer than gcc.
[9:33pm] was_: E.g., I just tested on sage/libs/cremona/*.pyx, and it takes < 4 seconds to run GCC and nearly 12 seconds to run Cython.
[9:33pm] mabshoff: ok
[9:33pm] was_: Maybe we should write Cython in Cython (ducks)
[9:34pm] mabshoff: my thoughts exactly.
[9:34pm] mabshoff: For 10% of the code it would probably be worth it for performance reasons.
[9:34pm] mabshoff: Maybe you should suggest it to Robert.
[9:34pm] was_: But from the point of view of parallel sage -br, it just means we need to do that in parallel too...
[9:34pm] was_: I.e., both are worth doing.
[9:35pm] mabshoff: Yep,.
[9:35pm] was_: First should be the gcc code.

Attachments (13)

trac_1261_extcode.patch (105.8 KB) - added by Gary Furnish 15 years ago.
Pbuild extcode repo
trac_1261_extcode_2.patch (5.5 KB) - added by Gary Furnish 15 years ago.
This file is to be applied on top of the first patch.
trac_1261_extcode_3.patch (5.3 KB) - added by Gary Furnish 15 years ago.
Apply on top of the 2nd patch
trac_1261_extcode_4.patch (2.6 KB) - added by Gary Furnish 15 years ago.
apply over 3rd patch
trac_1261_extcode_5.patch (20.1 KB) - added by Gary Furnish 15 years ago.
trac_1261_extcode_6.patch (5.5 KB) - added by Gary Furnish 14 years ago.
darwin
trac_1261_extcode_7b.patch (50.7 KB) - added by Gary Furnish 14 years ago.
trac_1261_extcode_8.patch (917 bytes) - added by Gary Furnish 14 years ago.
trac_1261_extcode_9.patch (1.2 KB) - added by Gary Furnish 14 years ago.
trac_1261_extcode_10.patch (3.0 KB) - added by Gary Furnish 14 years ago.
trac_1261_local_2.2.patch (673 bytes) - added by Gary Furnish 14 years ago.
trac_1261_extcode_11.patch (29.9 KB) - added by Gary Furnish 14 years ago.
trac_1261_local1.patch (3.2 KB) - added by Gary Furnish 14 years ago.
SAGE_PBUILD OPTION

Download all attachments as: .zip

Change History (38)

comment:1 Changed 15 years ago by Gary Furnish

Milestone: sage-featuresage-2.10.3
Owner: changed from was* to Gary Furnish
Status: newassigned

comment:2 Changed 15 years ago by Gary Furnish

The plan is to replace python distutils with scons for cython, etc.

comment:3 Changed 15 years ago by Gary Furnish

I built a new build system for this as scons was inadequate. An alpha version is available that does parallel build all (so it is a full rebuild every time). Set SAGE_BUILD_THREADS to set the number of threads, which defaults to 4. Download at: http://sage.math.washington.edu/home/gfurnish/misc/build.tar

Changed 15 years ago by Gary Furnish

Attachment: trac_1261_extcode.patch added

Pbuild extcode repo

comment:4 Changed 15 years ago by Gary Furnish

PBuild for extcode requires glib to work correctly. Obtain at #2436.

comment:5 Changed 15 years ago by Gary Furnish

Milestone: sage-3.0sage-2.11

comment:6 Changed 15 years ago by Gary Furnish

Summary: parallel "sage -br"[With patch, needs lots of review] parallel "sage -br"

Changed 15 years ago by Gary Furnish

Attachment: trac_1261_extcode_2.patch added

This file is to be applied on top of the first patch.

Changed 15 years ago by Gary Furnish

Attachment: trac_1261_extcode_3.patch added

Apply on top of the 2nd patch

comment:7 Changed 15 years ago by Michael Abshoff

A couple remarks:

  • the CPUID asm code is x86 specific
  • I don't understand what the option parsing code has to do in the build system?
  • class enviroment ought to be class environment
  • there ought to be spaces between defs
  • documentation in general is missing, but we agreed that it would be added past the review
  • how much code duplication is going on here with code like ptest?

I need to take a closer look to see what the code actually does.

Cheers,

Michael

comment:8 Changed 15 years ago by Gary Furnish

1) I fully realize the CPUID asm code is x86 specific, which is why its not fully implemented yet (or used anywhere) 2) The option parsing code doesn't have much to do with the build system other then I needed an option parser and this seemed like a good place to put it. It doesn't have any dependencies, so we could easily remove it. I envision a whole suite here though replacing sage-sage and friends, but maybe thats a bad idea? 3) Will run search and replace 4) Agreed 5) Right 6) There is a fair ammount of code duplication in ptest, which is why I plan on eventually removing ptest and building it into the build suite.

comment:9 Changed 15 years ago by Michael Abshoff

re option parser: sure, but that code ought to go into local/bin and then when you build code it should call the build system, not the other way around. It is also debatable how much of sage-sage can be written in python since Python is not a dependency of Sage.

re code duplication: eventually we need to factor out common code, but that is post-merge if you ask me.

re cpuid: yeah, I figured that much. DSage actually does some CPUID, too, via python's build in functions. So maybe there can be some common ground, too.

Cheers,

Michael

Changed 15 years ago by Gary Furnish

Attachment: trac_1261_extcode_4.patch added

apply over 3rd patch

comment:10 Changed 15 years ago by Michael Abshoff

Merged trac_1261_extcode.patch, trac_1261_extcode_2.patch, trac_1261_extcode_3.patch and trac_1261_extcode_4.patch in Sage 2.11.alpha1. Those patches are to the extcode repo and are still subject to review.

Cheers,

Michael

Changed 15 years ago by Gary Furnish

Attachment: trac_1261_extcode_5.patch added

comment:11 Changed 15 years ago by Michael Abshoff

trac_1261_extcode_5.patch looks good. Merged in Sage 2.11.rc0

Cheers,

Michael

comment:12 Changed 14 years ago by Michael Abshoff

Linker trouble on OSX 10.5:

cython --embed-positions --incref-local-binop -I. -o sage/matrix/matrix_rational_sparse.c 
sage/matrix/matrix_rational_sparse.pyx
Undefined symbols:
  "_PyType_IsSubtype", referenced from:
      _mpz_set_pylong in mpz_pylong.o
  "_PyExc_KeyboardInterrupt", referenced from:
      _PyExc_KeyboardInterrupt$non_lazy_ptr in interrupt.o
  "_PyErr_SetString", referenced from:
      _sig_handle_sigfpe in interrupt.o
      _sig_handle_sigbus in interrupt.o
      _sig_handle_sigsegv in interrupt.o
      _sage_signal_handler in interrupt.o
      _sage_signal_handler in interrupt.o
      _sage_signal_handler in interrupt.o
      _sage_signal_handler in interrupt.o
  "_PyTuple_New", referenced from:
      _init_global_empty_tuple in stdsage.o
  "_PyInt_FromLong", referenced from:
      _mpz_get_pyintlong in mpz_pylong.o
  "_PyObject_InitVar", referenced from:
      _mpz_get_pylong in mpz_pylong.o
  "_PyExc_RuntimeError", referenced from:
      _PyExc_RuntimeError$non_lazy_ptr in interrupt.o
  "__PyErr_BadInternalCall", referenced from:
      _mpz_set_pylong in mpz_pylong.o
  "_main", referenced from:
      start in crt1.10.5.o
  "_PyObject_Malloc", referenced from:
      _mpz_get_pylong in mpz_pylong.o
  "_PyLong_Type", referenced from:
      _PyLong_Type$non_lazy_ptr in mpz_pylong.o
ld: symbol(s) not found
collect2: ld returned 1 exit status
gcc /Users/mabshoff/sage-3.0.alpha1/devel/sage-main/c_lib/src/mpn_pylong.o 
/Users/mabshoff/sage-3.0.alpha1/devel/sage-main/c_lib/src/convert.o 
/Users/mabshoff/sage-3.0.alpha1/devel/sage-main/c_lib/src/stdsage.o 
/Users/mabshoff/sage-3.0.alpha1/devel/sage-main/c_lib/src/ntl_wrap.o 
/Users/mabshoff/sage-3.0.alpha1/devel/sage-main/c_lib/src/gmp_globals.o 
/Users/mabshoff/sage-3.0.alpha1/devel/sage-main/c_lib/src/interrupt.o 
/Users/mabshoff/sage-3.0.alpha1/devel/sage-main/c_lib/src/ZZ_pylong.o 
/Users/mabshoff/sage-3.0.alpha1/devel/sage-main/c_lib/src/mpz_pylong.o 
-O2 -g -shared -fno-strict-aliasing -L/Users/mabshoff/sage-3.0.alpha1/local/lib  
-lcsage  -lntl  -lgmp  -lpari  -lstdc++  -lntl  -o 
/Users/mabshoff/sage-3.0.alpha1/devel/sage-main/c_lib/libcsage.so
cython --embed-positions --incref-local-binop -I. -o sage/libs/ntl/ntl_GF2X.c 
sage/libs/ntl/ntl_GF2X.pyx

The output should not be called "libcsage.so", but "libcsage.dylib" on OSX. You also seem be be missing libpython.

Cheers,

Michael

Changed 14 years ago by Gary Furnish

Attachment: trac_1261_extcode_6.patch added

darwin

comment:13 Changed 14 years ago by Michael Abshoff

trac_1261_extcode_6.patch looks good to me. Merged in Sage 3.0.alpha2

comment:14 Changed 14 years ago by Michael Abshoff

#2346 touches setup.py:

diff -r 767784c4ad52 -r 4f98891c4496 setup.py
--- a/setup.py  Sat Apr 05 22:40:57 2008 -0500
+++ b/setup.py  Sat Apr 05 22:55:32 2008 -0700
@@ -1391,6 +1391,7 @@ code = setup(name        = 'sage',
                      'sage.schemes.hyperelliptic_curves',

                      'sage.server',
+                     'sage.server.simple',
                      'sage.server.server1',
                      'sage.server.notebook',
                      'sage.server.notebook.compress',

But since this is only packages I guess pbuild is not affected.

Cheers,

Michael

Changed 14 years ago by Gary Furnish

Attachment: trac_1261_extcode_7b.patch added

Changed 14 years ago by Gary Furnish

Attachment: trac_1261_extcode_8.patch added

comment:15 Changed 14 years ago by Michael Abshoff

Merged trac_1261_extcode_7b.patch and trac_1261_extcode_8.patch into Sage 3.0.alpha3

comment:16 Changed 14 years ago by Michael Abshoff

trac_1261_local_1b.patch is no good as is. I will also not switch the default build system to pbuild without

  • some more testing
  • any documentation since people who add extensions do need to know how to do it

Cheers,

Michael

Changed 14 years ago by Gary Furnish

Attachment: trac_1261_extcode_9.patch added

comment:17 Changed 14 years ago by Michael Abshoff

Merged trac_1261_extcode_9.patch in Sage 3.0.alpha3

Changed 14 years ago by Gary Furnish

Attachment: trac_1261_extcode_10.patch added

Changed 14 years ago by Gary Furnish

Attachment: trac_1261_local_2.2.patch added

comment:18 Changed 14 years ago by Gary Furnish

Ignore the duplicate local_2 patch

comment:19 Changed 14 years ago by Michael Abshoff

FYI: #2394 add a new extension to setup.py

Cheers,

Michael

comment:20 Changed 14 years ago by Michael Abshoff

Merged trac_1261_extcode_10.patch in Sage 3.0.rc0

Changed 14 years ago by Gary Furnish

Attachment: trac_1261_extcode_11.patch added

comment:21 Changed 14 years ago by Michael Abshoff

Resolution: fixed
Status: assignedclosed

Merged trac_1261_extcode_11.patch in Sage 3.0.1.alpha1

comment:22 Changed 14 years ago by Michael Abshoff

Resolution: fixed
Status: closedreopened

oops, I didn't mean to close this ;(

Changed 14 years ago by Gary Furnish

Attachment: trac_1261_local1.patch added

SAGE_PBUILD OPTION

comment:23 Changed 14 years ago by Michael Abshoff

Merged trac_1261_local1.patch in Sage 3.0.1.alpha1. "sage -b" in its various versions as well as "sage -clone" keeps working with SAGE_PBUILD=no, so more testing with SAGE_PBUILD=yes is needed.

Cheers,

Michael

comment:24 Changed 14 years ago by Michael Abshoff

Summary: [With patch, needs lots of review] parallel "sage -br"[With patch, positive review] parallel "sage -br"

The patch set works for me. While there are still potential issues with more exotic, but not yet supported arches this is good to go. Positive review. Follow up will be more documentation, but since this ticket is already so crowded those will be new tickets.

Cheers,

Michael

comment:25 Changed 14 years ago by Michael Abshoff

Resolution: fixed
Status: reopenedclosed

Merged in Sage 3.0.1.alpha1

Note: See TracTickets for help on using tickets.