Opened 6 years ago
Closed 5 years ago
#22462 closed enhancement (fixed)
Use ContainChildren to implement p_iter_fork
Reported by:  Jeroen Demeyer  Owned by:  

Priority:  major  Milestone:  sage8.1 
Component:  misc  Keywords:  
Cc:  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Sébastien Labbé 
Report Upstream:  N/A  Work issues:  
Branch:  75e41db (Commits, GitHub, GitLab)  Commit:  75e41dbeeb01de939ac14699b18fef270e9edb67 
Dependencies:  Stopgaps: 
Description (last modified by )
This is preparation for #15585.
Change History (26)
comment:1 Changed 6 years ago by
Description:  modified (diff) 

comment:2 Changed 6 years ago by
Branch:  → u/jdemeyer/use_containchildren_to_implement_p_iter_fork 

comment:3 Changed 6 years ago by
Commit:  → 62f870a79690c7a3ed57e7ebc3cf1e83774d355a 

Status:  new → needs_review 
comment:4 Changed 6 years ago by
Description:  modified (diff) 

comment:5 Changed 5 years ago by
Status:  needs_review → needs_work 

With the patch, on top of 8.0.rc0, make start
works. But ./sage
yields:
$ sage ┌────────────────────────────────────────────────────────────────────┐ │ SageMath version 8.0.rc0, Release Date: 20170629 │ │ Type "notebook()" for the browserbased notebook interface. │ │ Type "help()" for help. │ └────────────────────────────────────────────────────────────────────┘ ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ ┃ Warning: this is a prerelease version, and it may be unstable. ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ ********************************************************************** Oops, Sage crashed. We do our best to make it stable, but...
The last part of the crash report is:
/home/slabbe/GitBox/sage/local/lib/python2.7/sitepackages/sage/parallel/use_fork.py in <module>() 3 """ 4 5 #***************************************************************************** 6 # Copyright (C) 2010 William Stein <wstein@gmail.com> 7 # 8 # This program is free software: you can redistribute it and/or modify 9 # it under the terms of the GNU General Public License as published by 10 # the Free Software Foundation, either version 2 of the License, or 11 # (at your option) any later version. 12 # http://www.gnu.org/licenses/ 13 #***************************************************************************** 14 15 from __future__ import absolute_import, print_function 16 17 from cysignals.alarm import AlarmInterrupt, alarm, cancel_alarm > 18 from .safefork import ContainChildren global safefork = undefined global ContainChildren = undefined 19 from sage.misc.misc import walltime 20 21 22 class WorkerData(object): 23 """ 24 Simple class which stores data about a running ``p_iter_fork`` 25 worker. 26 27 This just stores three attributes: 28 29  ``input``: the input value used by this worker 30 31  ``starttime``: the walltime when this worker started 32 33  ``failure``: an optional message indicating the kind of failure ImportError: cannot import name ContainChildren *************************************************************************** History of session input: *** Last line of input (may not be in above history):
comment:6 Changed 5 years ago by
Looking at the log
commit 03c5e01 Author: Erik M. Bray <erik.bray@lri.fr> Date: Thu Nov 24 12:45:07 2016 +0000 Move the terminate and ContainChildren context managers into a new sage.interfaces.process module
which was done in #21206.
comment:7 Changed 5 years ago by
Commit:  62f870a79690c7a3ed57e7ebc3cf1e83774d355a → 8f7ff57275df3c8188c5186846f9a25c90105e5e 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
8f7ff57  Use ContainChildren to implement p_iter_fork

comment:8 Changed 5 years ago by
Milestone:  sage7.6 → sage8.1 

Status:  needs_work → needs_review 
Rebased to 8.0.rc0
comment:9 followup: 10 Changed 5 years ago by
On Ubuntu 16.04, with this branch on top of 8.0.rc0, and using optional=ccache,cmake,cryptominisat,mpir,python2,sage
, I get after MAKE='make j8' make ptestlong
:
 sage t long warnlong 60.8 src/sage/homology/simplicial_complex.py # 1 doctest failed sage t long warnlong 60.8 src/sage/sat/boolean_polynomials.py # 1 doctest failed 
which means this ticket does not create new doctest problem (I get the same two errors on 8.0.rc0) but also means that it does not solve #15585.
comment:10 followup: 11 Changed 5 years ago by
Replying to slabbe:
 sage t long warnlong 60.8 src/sage/homology/simplicial_complex.py # 1 doctest failed sage t long warnlong 60.8 src/sage/sat/boolean_polynomials.py # 1 doctest failed 
Do you have the actual test failure?
which means this ticket does not create new doctest problem (I get the same two errors on 8.0.rc0) but also means that it does not solve #15585.
Even then, I still think that this ticket is a good idea.
comment:11 followup: 14 Changed 5 years ago by
Replying to jdemeyer:
Replying to slabbe:
 sage t long warnlong 60.8 src/sage/homology/simplicial_complex.py # 1 doctest failed sage t long warnlong 60.8 src/sage/sat/boolean_polynomials.py # 1 doctest failed Do you have the actual test failure?
Yes:
sage t long warnlong 60.8 src/sage/homology/simplicial_complex.py ********************************************************************** File "src/sage/homology/simplicial_complex.py", line 2854, in sage.homology.simplicial_complex.SimplicialComplex.is_cohen_macaulay Failed example: X.is_cohen_macaulay(ZZ) Expected: False Got: [Errno 2] No such file or directory: '/home/slabbe/.sage/temp/miami/8475/dir_aMATb0/8618.out' False ********************************************************************** 1 item had failures: 1 of 8 in sage.homology.simplicial_complex.SimplicialComplex.is_cohen_macaulay [598 tests, 1 failure, 4.88 s] sage t long warnlong 60.8 src/sage/sat/boolean_polynomials.py ********************************************************************** File "src/sage/sat/boolean_polynomials.py", line 89, in sage.sat.boolean_polynomials.solve Failed example: s = solve_sat(F, s_verbosity=1, c_max_vars_sparse=4, c_cutting_number=8) # optional  cryptominisat Expected: c > ... ... Got: c [consolidate] T: 0.00 c Found matrixes: 0 T: 0.00 Tout: N ********************************************************************** 1 item had failures: 1 of 22 in sage.sat.boolean_polynomials.solve [27 tests, 1 failure, 0.76 s]
Even then, I still think that this ticket is a good idea.
Can you update the description of the ticket accordingly?
comment:12 Changed 5 years ago by
Reviewers:  → Sébastien Labbé 

Status:  needs_review → positive_review 
I get All tests passed
with make testlong
run serially.
comment:13 Changed 5 years ago by
Status:  positive_review → needs_work 

Oups!
$ sage coverage src/sage/parallel/use_fork.py  SCORE src/sage/parallel/use_fork.py: 75.0% (3 of 4) Missing documentation: * line 35: def __init__(self, input, starttime=None, failure="") Possibly wrong (function name doesn't occur in doctests): * line 232: def _subprocess(self, f, dir, x) 
comment:14 Changed 5 years ago by
Replying to slabbe:
sage t long warnlong 60.8 src/sage/homology/simplicial_complex.py ********************************************************************** File "src/sage/homology/simplicial_complex.py", line 2854, in sage.homology.simplicial_complex.SimplicialComplex.is_cohen_macaulay Failed example: X.is_cohen_macaulay(ZZ) Expected: False Got: [Errno 2] No such file or directory: '/home/slabbe/.sage/temp/miami/8475/dir_aMATb0/8618.out' False **********************************************************************
Could you please doublecheck that this is really the test result with this branch applied? I ask because it looks like an exception is being printed. As far as I can see, if verbose = False
(which is the case here), that only happens with the old code.
comment:15 Changed 5 years ago by
Commit:  8f7ff57275df3c8188c5186846f9a25c90105e5e → a4dddccdf11f9028cace629290b8264bca01b22e 

Branch pushed to git repo; I updated commit sha1. New commits:
a4dddcc  Further fixes to use_fork

comment:16 followup: 17 Changed 5 years ago by
Status:  needs_work → needs_review 

As I mentioned, I suspect that maybe you didn't test the right branch. Please test again.
comment:17 Changed 5 years ago by
Replying to jdemeyer:
As I mentioned, I suspect that maybe you didn't test the right branch. Please test again.
No problem, I just launched the make ptestlong.
comment:18 Changed 5 years ago by
After merging the new branch on 8.0.rc0 again, MAKE='make j8' make pteslong
finishes with:
 sage t long warnlong 30.2 src/sage/homology/simplicial_complex.py # 1 doctest failed 
The failure has changed:
sage t long warnlong 30.2 src/sage/homology/simplicial_complex.py ********************************************************************** File "src/sage/homology/simplicial_complex.py", line 2854, in sage.homology.simplicial_complex.SimplicialComplex.is_cohen_macaulay Failed example: X.is_cohen_macaulay(ZZ) Expected: False Got: Exception raised by child process with pid=27028: Traceback (most recent call last): File "/home/slabbe/GitBox/sage/local/lib/python2.7/sitepackages/sage/parallel/use_fork.py", line 165, in __call__ self._subprocess(f, dir, *v0) File "/home/slabbe/GitBox/sage/local/lib/python2.7/sitepackages/sage/parallel/use_fork.py", line 294, in _subprocess save(value, sobj, compress=False) File "sage/structure/sage_object.pyx", line 1164, in sage.structure.sage_object.save (build/cythonized/sage/structure/sage_object.c:13871) open(process(filename), 'wb').write(s) IOError: [Errno 2] No such file or directory: '/home/slabbe/.sage/temp/miami/26912/dir_4FVAPf/27028.sobj' Exception raised by child process with pid=27026: Traceback (most recent call last): File "/home/slabbe/GitBox/sage/local/lib/python2.7/sitepackages/sage/parallel/use_fork.py", line 165, in __call__ self._subprocess(f, dir, *v0) File "/home/slabbe/GitBox/sage/local/lib/python2.7/sitepackages/sage/parallel/use_fork.py", line 294, in _subprocess save(value, sobj, compress=False) File "sage/structure/sage_object.pyx", line 1164, in sage.structure.sage_object.save (build/cythonized/sage/structure/sage_object.c:13871) open(process(filename), 'wb').write(s) IOError: [Errno 2] No such file or directory: '/home/slabbe/.sage/temp/miami/26912/dir_4FVAPf/27026.sobj' Exception raised by child process with pid=27029: Traceback (most recent call last): File "/home/slabbe/GitBox/sage/local/lib/python2.7/sitepackages/sage/parallel/use_fork.py", line 165, in __call__ self._subprocess(f, dir, *v0) File "/home/slabbe/GitBox/sage/local/lib/python2.7/sitepackages/sage/parallel/use_fork.py", line 294, in _subprocess save(value, sobj, compress=False) File "sage/structure/sage_object.pyx", line 1164, in sage.structure.sage_object.save (build/cythonized/sage/structure/sage_object.c:13871) open(process(filename), 'wb').write(s) IOError: [Errno 2] No such file or directory: '/home/slabbe/.sage/temp/miami/26912/dir_4FVAPf/27029.sobj' Exception raised by child process with pid=27027: Traceback (most recent call last): File "/home/slabbe/GitBox/sage/local/lib/python2.7/sitepackages/sage/parallel/use_fork.py", line 165, in __call__ self._subprocess(f, dir, *v0) File "/home/slabbe/GitBox/sage/local/lib/python2.7/sitepackages/sage/parallel/use_fork.py", line 294, in _subprocess save(value, sobj, compress=False) File "sage/structure/sage_object.pyx", line 1164, in sage.structure.sage_object.save (build/cythonized/sage/structure/sage_object.c:13871) open(process(filename), 'wb').write(s) IOError: [Errno 2] No such file or directory: '/home/slabbe/.sage/temp/miami/26912/dir_4FVAPf/27027.sobj' False ********************************************************************** 1 item had failures: 1 of 8 in sage.homology.simplicial_complex.SimplicialComplex.is_cohen_macaulay [598 tests, 1 failure, 5.19 s]
comment:20 Changed 5 years ago by
Well, it's not excellent that the failure is still there but it's good to see a traceback instead of just the error message being printed.
comment:21 Changed 5 years ago by
Description:  modified (diff) 

Status:  needs_work → needs_review 
And thanks to the traceback, the failure becomes obvious: the working directory of the workers is removed before the workers are killed. So a worker can try to write its result in the working directory after it has been deleted. I will deal with that in #15535.
comment:22 Changed 5 years ago by
Status:  needs_review → needs_work 

The init method has still no doc neither doctests.
comment:23 Changed 5 years ago by
Status:  needs_work → needs_review 

The __init__
method is too trivial... there is no meaningful doc to add which is not already in the class doc.
comment:24 followup: 25 Changed 5 years ago by
Branch:  u/jdemeyer/use_containchildren_to_implement_p_iter_fork → u/slabbe/22462 

Commit:  a4dddccdf11f9028cace629290b8264bca01b22e → 75e41dbeeb01de939ac14699b18fef270e9edb67 
I am sorry but I prefer to make it 100% coverage to avoid the noise of seeing 75% forever for that file on every run of sage coverage. So I added a commit. If you agree with my commit, I let you change the status to positive review.
New commits:
75e41db  adding docstring to WorkerData.__init__

comment:25 Changed 5 years ago by
Status:  needs_review → positive_review 

Replying to slabbe:
If you agree with my commit, I let you change the status to positive review.
I do not agree.
Despite that, I'm still setting it to positive_review in order to move forward.
comment:26 Changed 5 years ago by
Branch:  u/slabbe/22462 → 75e41dbeeb01de939ac14699b18fef270e9edb67 

Resolution:  → fixed 
Status:  positive_review → closed 
New commits:
Use ContainChildren to implement p_iter_fork