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: sage-8.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:

Status badges

Description (last modified by Jeroen Demeyer)

This is preparation for #15585.

Change History (26)

comment:1 Changed 6 years ago by Jeroen Demeyer

Description: modified (diff)

comment:2 Changed 6 years ago by Jeroen Demeyer

Branch: u/jdemeyer/use_containchildren_to_implement_p_iter_fork

comment:3 Changed 6 years ago by Jeroen Demeyer

Commit: 62f870a79690c7a3ed57e7ebc3cf1e83774d355a
Status: newneeds_review

New commits:

62f870aUse ContainChildren to implement p_iter_fork

comment:4 Changed 6 years ago by Jeroen Demeyer

Description: modified (diff)

comment:5 Changed 5 years ago by Sébastien Labbé

Status: needs_reviewneeds_work

With the patch, on top of 8.0.rc0, make start works. But ./sage yields:

$ sage
┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 8.0.rc0, Release Date: 2017-06-29                 │
│ Type "notebook()" for the browser-based 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/site-packages/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 Travis Scrimshaw

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 git

Commit: 62f870a79690c7a3ed57e7ebc3cf1e83774d355a8f7ff57275df3c8188c5186846f9a25c90105e5e

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

8f7ff57Use ContainChildren to implement p_iter_fork

comment:8 Changed 5 years ago by Jeroen Demeyer

Milestone: sage-7.6sage-8.1
Status: needs_workneeds_review

Rebased to 8.0.rc0

comment:9 Changed 5 years ago by Sébastien Labbé

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 --warn-long 60.8 src/sage/homology/simplicial_complex.py  # 1 doctest failed
sage -t --long --warn-long 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.

Last edited 5 years ago by Sébastien Labbé (previous) (diff)

comment:10 in reply to:  9 ; Changed 5 years ago by Jeroen Demeyer

Replying to slabbe:

----------------------------------------------------------------------
sage -t --long --warn-long 60.8 src/sage/homology/simplicial_complex.py  # 1 doctest failed
sage -t --long --warn-long 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 in reply to:  10 ; Changed 5 years ago by Sébastien Labbé

Replying to jdemeyer:

Replying to slabbe:

----------------------------------------------------------------------
sage -t --long --warn-long 60.8 src/sage/homology/simplicial_complex.py  # 1 doctest failed
sage -t --long --warn-long 60.8 src/sage/sat/boolean_polynomials.py  # 1 doctest failed
----------------------------------------------------------------------

Do you have the actual test failure?

Yes:

sage -t --long --warn-long 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 --warn-long 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 T-out: 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 Sébastien Labbé

Reviewers: Sébastien Labbé
Status: needs_reviewpositive_review

I get All tests passed with make testlong run serially.

comment:13 Changed 5 years ago by Sébastien Labbé

Status: positive_reviewneeds_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 in reply to:  11 Changed 5 years ago by Jeroen Demeyer

Replying to slabbe:

sage -t --long --warn-long 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 double-check 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 git

Commit: 8f7ff57275df3c8188c5186846f9a25c90105e5ea4dddccdf11f9028cace629290b8264bca01b22e

Branch pushed to git repo; I updated commit sha1. New commits:

a4dddccFurther fixes to use_fork

comment:16 Changed 5 years ago by Jeroen Demeyer

Status: needs_workneeds_review

As I mentioned, I suspect that maybe you didn't test the right branch. Please test again.

comment:17 in reply to:  16 Changed 5 years ago by Sébastien Labbé

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 Sébastien Labbé

After merging the new branch on 8.0.rc0 again, MAKE='make -j8' make pteslong finishes with:

----------------------------------------------------------------------
sage -t --long --warn-long 30.2 src/sage/homology/simplicial_complex.py  # 1 doctest failed
----------------------------------------------------------------------

The failure has changed:

sage -t --long --warn-long 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/site-packages/sage/parallel/use_fork.py", line 165, in __call__
        self._subprocess(f, dir, *v0)
      File "/home/slabbe/GitBox/sage/local/lib/python2.7/site-packages/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/site-packages/sage/parallel/use_fork.py", line 165, in __call__
        self._subprocess(f, dir, *v0)
      File "/home/slabbe/GitBox/sage/local/lib/python2.7/site-packages/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/site-packages/sage/parallel/use_fork.py", line 165, in __call__
        self._subprocess(f, dir, *v0)
      File "/home/slabbe/GitBox/sage/local/lib/python2.7/site-packages/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/site-packages/sage/parallel/use_fork.py", line 165, in __call__
        self._subprocess(f, dir, *v0)
      File "/home/slabbe/GitBox/sage/local/lib/python2.7/site-packages/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:19 Changed 5 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work

Excellent!

comment:20 Changed 5 years ago by Jeroen Demeyer

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 Jeroen Demeyer

Description: modified (diff)
Status: needs_workneeds_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 Sébastien Labbé

Status: needs_reviewneeds_work

The init method has still no doc neither doctests.

comment:23 Changed 5 years ago by Jeroen Demeyer

Status: needs_workneeds_review

The __init__ method is too trivial... there is no meaningful doc to add which is not already in the class doc.

comment:24 Changed 5 years ago by Sébastien Labbé

Branch: u/jdemeyer/use_containchildren_to_implement_p_iter_forku/slabbe/22462
Commit: a4dddccdf11f9028cace629290b8264bca01b22e75e41dbeeb01de939ac14699b18fef270e9edb67

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:

75e41dbadding docstring to WorkerData.__init__

comment:25 in reply to:  24 Changed 5 years ago by Jeroen Demeyer

Status: needs_reviewpositive_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 Volker Braun

Branch: u/slabbe/2246275e41dbeeb01de939ac14699b18fef270e9edb67
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.