the timeout option is not working correctly in parallel computing

[mderickx]

With a timeout of 5 seconds I should not be able to perform a sleep of 40 seconds!

sage: from time import sleep
sage: f=parallel(ncpus=4,timeout=5,verbose=True)
sage: g=f(sleep)
sage: time list(g([5,10,20,40]))
[(((5,), {}), None), (((10,), {}), None), (((20,), {}), None), (((40,), {}), None)]
Time: CPU 0.02 s, Wall: 40.07 s

[leif]

Is this really an issue?

I guess the timeout simply sets a SIGALRM, which the sleep() function(s) override, but I may be wrong.

[leif]

P.S.:

If you use p_iter="fork" (the only variant that's said to support timeout), apparently indeed only ncpus-1 child processes are created, so it seems to be as I guessed.

Depending on the selection / order of arguments, you may well get timeouts for [perhaps only some of] the child processes (i.e. they'll get killed), though not after the time you'd expect.

The behaviour is non-deterministic though, for whatever reason. (Try running the time command repeatedly, i.e. within a loop.)

[leif]

Oh, this is a more funny one (and not due to what I first guessed).

Looking at the code, it does spawn ncpus processes, but there's a fat bug in the code (re)computing how long to wait the next time (the call to signal.alarm()):

                    if timeout:
                        def mysig(a,b):
                            raise RuntimeError, "SIGALRM"
                        oldest = min([X[1] for X in workers.values()])
                        signal.signal(signal.SIGALRM, mysig)
                        signal.alarm(int(walltime() - oldest)+1)

(X[1] is the wall time each child process was forked / started.)

This code is executed repeatedly; if wait() was interrupted by a RuntimeError triggered by SIGALRM, it is checked whether any of the child processes timed out, and if so, they get killed.

[leif]

Sage library patch. Corrects time to wait for child processes (before they get killed) in the parallel fork decorator. Based on Sage 4.7.1.rc0.

[leif]

A trivial patch is up.

[vbraun]

Yes its definitely better to have the timeout depend on the timeout variable :-)

[leif]

Replying to vbraun:

Yes its definitely better to have the timeout depend on the timeout variable :-)

:D Just wondering if we should add a doctest for that, but now it has already positive review...

I was going to add one similar to Maarten's example (with some parallel sleeping processes, the default of ncpus, and a timeout of about 5 seconds), marking it # long time.

[mderickx]

I already had a patch sorry guy's for making you do double effort but thanks for the quick fix :). I found this bug by reading the source code since I was trying to understand what they where doing. BTW I didn't respond earlier because the mailing system of the trac server was malfunctioning and didn't know others where working on it. The mailing server should work now. At least this message will test that for me :)

[mderickx]

Ok trac wasn't fixed yet :(. Another test

[leif]

Replying to mderickx:

Ok trac wasn't fixed yet :(. Another test

For the record, I did get this one. [Another test :) ]