search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.3k stars 447 forks source link

Use ContainChildren to implement p_iter_fork #22462

Closed jdemeyer closed 7 years ago

jdemeyer commented 7 years ago

This is preparation for #15585.

Component: misc

Author: Jeroen Demeyer

Branch/Commit: 75e41db

Reviewer: Sébastien Labbé

Issue created by migration from https://trac.sagemath.org/ticket/22462

jdemeyer commented 7 years ago

Description changed:

--- 
+++ 
@@ -1 +1 @@
-This might fix #15585.
+This might fix #15585. But given that #15585 is non-deterministic, that is hard to know for sure.
jdemeyer commented 7 years ago

Branch: u/jdemeyer/use_containchildren_to_implement_p_iter_fork

jdemeyer commented 7 years ago

New commits:

62f870aUse ContainChildren to implement p_iter_fork
jdemeyer commented 7 years ago

Commit: 62f870a

jdemeyer commented 7 years ago

Description changed:

--- 
+++ 
@@ -1 +1,3 @@
 This might fix #15585. But given that #15585 is non-deterministic, that is hard to know for sure.
+
+This ticket also does some clean-up of `p_iter_fork`.
seblabbe commented 7 years ago
comment:5

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):
tscrim commented 7 years ago
comment:6

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.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

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

8f7ff57Use ContainChildren to implement p_iter_fork
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 62f870a to 8f7ff57

jdemeyer commented 7 years ago
comment:8

Rebased to 8.0.rc0

seblabbe commented 7 years ago
comment:9

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.

jdemeyer commented 7 years ago
comment:10

Replying to @seblabbe:

----------------------------------------------------------------------
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.

seblabbe commented 7 years ago
comment:11

Replying to @jdemeyer:

Replying to @seblabbe:

----------------------------------------------------------------------
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?

seblabbe commented 7 years ago

Reviewer: Sébastien Labbé

seblabbe commented 7 years ago
comment:12

I get All tests passed with make testlong run serially.

seblabbe commented 7 years ago
comment:13

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)
------------------------------------------------------------------------
jdemeyer commented 7 years ago
comment:14

Replying to @seblabbe:

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.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 8f7ff57 to a4dddcc

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

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

a4dddccFurther fixes to use_fork
jdemeyer commented 7 years ago
comment:16

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

seblabbe commented 7 years ago
comment:17

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.

seblabbe commented 7 years ago
comment:18

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]
jdemeyer commented 7 years ago
comment:19

Excellent!

jdemeyer commented 7 years ago
comment:20

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.

jdemeyer commented 7 years ago
comment:21

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.

jdemeyer commented 7 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1 @@
-This might fix #15585. But given that #15585 is non-deterministic, that is hard to know for sure.
-
-This ticket also does some clean-up of `p_iter_fork`.
+This is preparation for #15585.
seblabbe commented 7 years ago
comment:22

The init method has still no doc neither doctests.

jdemeyer commented 7 years ago
comment:23

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

seblabbe commented 7 years ago
comment:24

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__
seblabbe commented 7 years ago

Changed branch from u/jdemeyer/use_containchildren_to_implement_p_iter_fork to u/slabbe/22462

seblabbe commented 7 years ago

Changed commit from a4dddcc to 75e41db

jdemeyer commented 7 years ago
comment:25

Replying to @seblabbe:

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.

vbraun commented 7 years ago

Changed branch from u/slabbe/22462 to 75e41db