Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#9501 closed enhancement (fixed)

Make an @fork decorator

Reported by: was Owned by: jason
Priority: minor Milestone: sage-4.5.2
Component: misc Keywords:
Cc: mvngu Merged in: sage-4.5.2.alpha1
Authors: William Stein Reviewers: Martin Albrecht
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

Simon King mentioned that sometimes his code crashes/leaks/etc. So make it so one can do:

@fork
def f(x,y,z,...):
    ...

and then f gets computed in a blocking forked process, and the result is returned via pickling. This is 100% to thwart mem leaks, segfaults, and guaranteed timeout possibility. This could be basically just a light wrapper around @parallel(1). Also, make a global flag to turn this off, so @fork does nothing.

Attachments (2)

trac_9501.patch (9.9 KB) - added by was 11 years ago.
trac_9501.2.patch (10.0 KB) - added by was 11 years ago.
new version with fixed warnings.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 11 years ago by was

Keep in mind #8410, though this doesn't depend on that.

See http://sagenb.org/home/pub/2248/ for a demo.

comment:2 Changed 11 years ago by was

  • Status changed from new to needs_review

The code is a little longer than necessary, since:

  • I improved the documentation and doctesting to get coverage to 100% (it wasn't really before).
  • I implemented an option to not kill the interfaces, but it's not used. It could be useful for some users.

comment:3 Changed 11 years ago by malb

File "/mnt/usb1/scratch/malb/sage-4.4/devel/sage/sage/parallel/decorate.py", line 292:

    sage: g()

Expected:

    '10'

Got:

    [Errno 16] Device or resource busy: '/home/malb/.sage/temp/sage.math.washington.edu/29514/dir_0/.nfs000000000482d3cd00028d9f'

    '10'

**********************************************************************

File "/mnt/usb1/scratch/malb/sage-4.4/devel/sage/sage/parallel/decorate.py", line 303:

    sage: g()

Expected:

    'a'

Got:

    [Errno 16] Device or resource busy: '/home/malb/.sage/temp/sage.math.washington.edu/29514/dir_1/.nfs000000000482d3d300028da0'

    'a'


comment:4 Changed 11 years ago by malb

  • the timeout documentation should mention that 0 means Infinity
  • the fork decorator talks about parallel subprocesses (copy 'n' paste oversight)
  • I don't know whether the above failures are a real issue or not

If these are addressed I'm happy to give this a positive review.

comment:5 Changed 11 years ago by malb

William and I discussed the doctest failure and tried it again: we couldn't reproduce it so we assume it's okay for now, the machine (disk?) was just quite busy.

comment:6 Changed 11 years ago by malb

  • Status changed from needs_review to positive_review

comment:7 Changed 11 years ago by SimonKing

William, you just mentioned that you have not added a doctest illustrating that a segmentation fault is no disaster when one uses the fork decorator. But in the worksheet you link at, there is a segfaulting example. So, why not add such thing as a doctest?

comment:8 Changed 11 years ago by was

SimonKing? -- that's a good idea. I've refreshed the patch with this example.

Changed 11 years ago by was

comment:9 Changed 11 years ago by was

  • Status changed from positive_review to needs_work

comment:10 Changed 11 years ago by was

  • Status changed from needs_work to needs_review

comment:11 Changed 11 years ago by malb

  • Authors set to William Stein
  • Reviewers set to Martin Albrecht
  • Status changed from needs_review to positive_review

Patch looks good, tests passed.

comment:12 follow-up: Changed 11 years ago by kcrisman

  • Cc mvngu added
  • Status changed from positive_review to needs_work

I think the 'warning' sections need to be in proper REsT format, which I *think* (but won't guarantee) should look more like

.. warning::

or something like that. I'm cc:ing mvngu since he will know :)

comment:13 in reply to: ↑ 12 Changed 11 years ago by mvngu

Replying to kcrisman:

I think the 'warning' sections need to be in proper REsT format, which I *think* (but won't guarantee) should look more like

.. warning::

or something like that.

That's right. See this section of the Sphinx reference manual.

Changed 11 years ago by was

new version with fixed warnings.

comment:14 Changed 11 years ago by was

  • Status changed from needs_work to needs_review

comment:15 Changed 11 years ago by malb

Looks good, but the file doesn't seem to be included in the reference manual anyway.

comment:16 Changed 11 years ago by malb

  • Status changed from needs_review to positive_review

comment:17 Changed 11 years ago by ddrake

  • Merged in set to sage-4.5.2.alpha1
  • Resolution set to fixed
  • Status changed from positive_review to closed

Merged trac_9501.2.patch in 4.5.2.alpha1.

comment:18 Changed 11 years ago by mpatel

This ticket was backed out at #9616. The new ticket for merging this is #9631.

Note: See TracTickets for help on using tickets.