#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: |
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)
Change History (20)
comment:1 Changed 12 years ago by
comment:2 Changed 12 years ago by
- 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 12 years ago by
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 12 years ago by
- 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 12 years ago by
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 12 years ago by
- Status changed from needs_review to positive_review
comment:7 Changed 12 years ago by
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 12 years ago by
SimonKing? -- that's a good idea. I've refreshed the patch with this example.
Changed 12 years ago by
comment:9 Changed 12 years ago by
- Status changed from positive_review to needs_work
comment:10 Changed 12 years ago by
- Status changed from needs_work to needs_review
comment:11 Changed 12 years ago by
- Reviewers set to Martin Albrecht
- Status changed from needs_review to positive_review
Patch looks good, tests passed.
comment:12 follow-up: ↓ 13 Changed 12 years ago by
- 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 12 years ago by
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.
comment:14 Changed 12 years ago by
- Status changed from needs_work to needs_review
comment:15 Changed 12 years ago by
Looks good, but the file doesn't seem to be included in the reference manual anyway.
comment:16 Changed 12 years ago by
- Status changed from needs_review to positive_review
comment:17 Changed 12 years ago by
- 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.
Keep in mind #8410, though this doesn't depend on that.
See http://sagenb.org/home/pub/2248/ for a demo.