Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cython.parallel.threadid unavailable in function arguments #3594

Closed
Celelibi opened this issue May 9, 2020 · 6 comments
Closed

cython.parallel.threadid unavailable in function arguments #3594

Celelibi opened this issue May 9, 2020 · 6 comments

Comments

@Celelibi
Copy link
Contributor

Celelibi commented May 9, 2020

It looks like this is a valid cython code.

from cython import parallel
x = parallel.threadid()
print(x)

But this is not.

from cython import parallel
print(parallel.threadid())

This might be just a symptom of a larger issue. Many things are not possible with the cython module when compiled with cython, compared to when run with python. Like accessing its module object.

import cython
x = cython
@da-woods
Copy link
Contributor

da-woods commented May 9, 2020

print(parallel.threadid()) looks like a bug.


Not being able to assign the cython module isn't. The Cython module isn't really a proper module, should really only be cimported (a compile-time action), and doesn't have an actual module object associated with it. It can be imported so it's possible to write pure-Python code with optional Cython features.

If you were able to assign the cython module then people would do things like:

import cython

if something():
     x = cython
else:
     class Dummy:
         def cclass(self, var):
             return var
    x = Dummy()

@x.cclass
class C:
    pass

Suddenly you've made the choice of whether C is a cdef or regular class a runtime decision rather than a compile-time decision...

@scoder
Copy link
Contributor

scoder commented May 10, 2020

Cython explicitly tracks usage of its own special "modules" in the InterpretCompilerDirectives AST transformation in ParseTreeTransforms.py. PR welcome that adds support for this.

@Celelibi
Copy link
Contributor Author

Suddenly you've made the choice of whether C is a cdef or regular class a runtime decision rather than a compile-time decision...

That's true.
However, I wonder if that's the right way to go about it as there are more ways to try to trick the compiler. Like having an intermediate decorator calling cython.cclass. I guess Cython could benefit from having an explicit notion of "compile-time executable" code.

Cython explicitly tracks usage of its own special "modules" in the InterpretCompilerDirectives AST transformation in ParseTreeTransforms.py. PR welcome that adds support for this.

I gave it a go. I think the method ParallelRangeTransform.visit_CallNode lacks a call to self.visitchildren(node) in order to visit the functions arguments and not just the function name.

def visit_CallNode(self, node):
self.visit(node.function)
if not self.parallel_directive:
return node

But when I changed self.visit(node.function) to a self.visitchildren(node), then the compiler crash much later because parallelRangeNode doesn't have a method analyse_types. I guess the traversal shouldn't recurse into the argument of a prange.
When add a call to visitchildren only when the node is not a parallel directive, then some tests fail unexplicably. And I already spent way too much time trying to reproduce the issue outside of runtest.py without any success.
I give up.

@da-woods
Copy link
Contributor

@Celelibi As far as I can tell your "add a call to visitchildren only when the node is not a parallel directive" seems to work fine. Thanks for the pointer on what needed changing - I'm not sure why it wasn't working for you.

@Celelibi
Copy link
Contributor Author

@da-woods, for what it's worth, here's the log of runtest.py for on the tests "parallel" on your PR.

$ python3 ./runtests.py -vv --no-cpp parallel
Python 3.8.3rc1 (default, Apr 30 2020, 07:33:30) 
[GCC 9.3.0]

Running tests against Cython 3.0a3 879af4af720e18ad0546102cc4b9160e828a0999
Using Cython language level 2.
Backends: c

runTest (__main__.CythonCompileTestCase)
compiling (c/cy2) parallel_compile_float_rank ... ok
runTest (__main__.CythonCompileTestCase)
compiling (c/cy2) e_cython_parallel ... ok
runTest (__main__.CythonRunTestCase)
compiling (c/cy2) and running numpy_parallel ... test_parallel_numpy_arrays (numpy_parallel)
Doctest: numpy_parallel.test_parallel_numpy_arrays ... ok
runTest (__main__.CythonRunTestCase)
compiling (c/cy2) and running parallel ... 
#### 2020-05-14 13:09:33.803802
outer_parallel_section (parallel)
Doctest: parallel.outer_parallel_section ... ok
parallel_exc_replace (parallel)
Doctest: parallel.parallel_exc_replace ... ok
parallel_exceptions2 (parallel)
Doctest: parallel.parallel_exceptions2 ... ok
test_chunksize (parallel)
Doctest: parallel.test_chunksize ... FAIL
test_clean_temps (parallel)
Doctest: parallel.test_clean_temps ... ok
test_closure_parallel_privates (parallel)
Doctest: parallel.test_closure_parallel_privates ... ok
test_closure_parallel_with_gil (parallel)
Doctest: parallel.test_closure_parallel_with_gil ... ok
test_descending_prange (parallel)
Doctest: parallel.test_descending_prange ... ok
test_else_clause (parallel)
Doctest: parallel.test_else_clause ... ok
test_inner_private (parallel)
Doctest: parallel.test_inner_private ... ok
test_nan_init (parallel)
Doctest: parallel.test_nan_init ... ok
test_nested_break_continue (parallel)
Doctest: parallel.test_nested_break_continue ... ok
test_nogil_cdef_except_clause (parallel)
Doctest: parallel.test_nogil_cdef_except_clause ... ok
test_num_threads (parallel)
Doctest: parallel.test_num_threads ... ok
test_parallel (parallel)
Doctest: parallel.test_parallel ... ok
test_parallel_exc_cdef (parallel)
Doctest: parallel.test_parallel_exc_cdef ... ok
test_parallel_exc_cpdef (parallel)
Doctest: parallel.test_parallel_exc_cpdef ... ok
test_parallel_exc_nogil_swallow (parallel)
Doctest: parallel.test_parallel_exc_nogil_swallow ... ok
test_parallel_exceptions (parallel)
Doctest: parallel.test_parallel_exceptions ... ok
test_parallel_exceptions_unnested (parallel)
Doctest: parallel.test_parallel_exceptions_unnested ... ok
test_parallel_with_gil_continue_unnested (parallel)
Doctest: parallel.test_parallel_with_gil_continue_unnested ... ok
test_parallel_with_gil_return (parallel)
Doctest: parallel.test_parallel_with_gil_return ... ok
test_pointer_temps (parallel)
Doctest: parallel.test_pointer_temps ... ok
test_prange (parallel)
Doctest: parallel.test_prange ... FAIL
test_prange_break (parallel)
Doctest: parallel.test_prange_break ... ok
test_prange_continue (parallel)
Doctest: parallel.test_prange_continue ... ok
test_prange_in_with (parallel)
Doctest: parallel.test_prange_in_with ... ok
test_prange_matches_range (parallel)
Doctest: parallel.test_prange_matches_range ... ok
test_propagation (parallel)
Doctest: parallel.test_propagation ... ok
test_pure_mode (parallel)
Doctest: parallel.test_pure_mode ... ok
test_reassign_start_stop_step (parallel)
Doctest: parallel.test_reassign_start_stop_step ... ok
test_return (parallel)
Doctest: parallel.test_return ... ok
runTest (__main__.CythonRunTestCase)
compiling (c/cy2) and running parallel_swap_assign_T425 ... swap (parallel_swap_assign_T425)
Doctest: parallel_swap_assign_T425.swap ... ok
swap5 (parallel_swap_assign_T425)
Doctest: parallel_swap_assign_T425.swap5 ... ok
swap_attr_values (parallel_swap_assign_T425)
Doctest: parallel_swap_assign_T425.swap_attr_values ... ok
swap_cmp5 (parallel_swap_assign_T425)
Doctest: parallel_swap_assign_T425.swap_cmp5 ... ok
swap_list_items (parallel_swap_assign_T425)
Doctest: parallel_swap_assign_T425.swap_list_items ... ok
swap_py (parallel_swap_assign_T425)
Doctest: parallel_swap_assign_T425.swap_py ... ok
swap_recursive_attr_values (parallel_swap_assign_T425)
Doctest: parallel_swap_assign_T425.swap_recursive_attr_values ... ok
runTest (__main__.CythonRunTestCase)
compiling (c/cy2) and running pure_parallel ... prange_regression (pure_parallel)
Doctest: pure_parallel.prange_regression ... ok
prange_with_gil (pure_parallel)
Doctest: pure_parallel.prange_with_gil ... ok
prange_with_gil_call_nogil (pure_parallel)
Doctest: pure_parallel.prange_with_gil_call_nogil ... ok
run (__main__.PureDoctestTestCase)
running pure doctests in pure_parallel ... prange_regression (pure_doctest__pure_parallel)
Doctest: pure_doctest__pure_parallel.prange_regression ... ok
prange_with_gil (pure_doctest__pure_parallel)
Doctest: pure_doctest__pure_parallel.prange_with_gil ... ok
prange_with_gil_call_nogil (pure_doctest__pure_parallel)
Doctest: pure_doctest__pure_parallel.prange_with_gil_call_nogil ... ok
runTest (__main__.CythonRunTestCase)
compiling (c/cy2) and running sequential_parallel ... 
#### 2020-05-14 13:09:43.883730

=== C/C++ compiler error output: ===
sequential_parallel.c: In function ‘__pyx_pf_19sequential_parallel_58test_chunksize’:
sequential_parallel.c:17361:7: warning: variable ‘__pyx_t_3’ set but not used [-Wunused-but-set-variable]
17361 |   int __pyx_t_3;
      |       ^~~~~~~~~
sequential_parallel.c: In function ‘__pyx_pf_19sequential_parallel_68test_inner_private’:
sequential_parallel.c:19519:7: warning: variable ‘__pyx_t_4’ set but not used [-Wunused-but-set-variable]
19519 |   int __pyx_t_4;
      |       ^~~~~~~~~
====================================
outer_parallel_section (sequential_parallel)
Doctest: sequential_parallel.outer_parallel_section ... ok
parallel_exc_replace (sequential_parallel)
Doctest: sequential_parallel.parallel_exc_replace ... ok
parallel_exceptions2 (sequential_parallel)
Doctest: sequential_parallel.parallel_exceptions2 ... ok
test_chunksize (sequential_parallel)
Doctest: sequential_parallel.test_chunksize ... ok
test_clean_temps (sequential_parallel)
Doctest: sequential_parallel.test_clean_temps ... ok
test_closure_parallel_privates (sequential_parallel)
Doctest: sequential_parallel.test_closure_parallel_privates ... ok
test_closure_parallel_with_gil (sequential_parallel)
Doctest: sequential_parallel.test_closure_parallel_with_gil ... ok
test_descending_prange (sequential_parallel)
Doctest: sequential_parallel.test_descending_prange ... ok
test_else_clause (sequential_parallel)
Doctest: sequential_parallel.test_else_clause ... ok
test_inner_private (sequential_parallel)
Doctest: sequential_parallel.test_inner_private ... ok
test_nan_init (sequential_parallel)
Doctest: sequential_parallel.test_nan_init ... ok
test_nested_break_continue (sequential_parallel)
Doctest: sequential_parallel.test_nested_break_continue ... ok
test_nogil_cdef_except_clause (sequential_parallel)
Doctest: sequential_parallel.test_nogil_cdef_except_clause ... ok
test_parallel_exc_cdef (sequential_parallel)
Doctest: sequential_parallel.test_parallel_exc_cdef ... ok
test_parallel_exc_cpdef (sequential_parallel)
Doctest: sequential_parallel.test_parallel_exc_cpdef ... ok
test_parallel_exc_nogil_swallow (sequential_parallel)
Doctest: sequential_parallel.test_parallel_exc_nogil_swallow ... ok
test_parallel_exceptions (sequential_parallel)
Doctest: sequential_parallel.test_parallel_exceptions ... ok
test_parallel_exceptions_unnested (sequential_parallel)
Doctest: sequential_parallel.test_parallel_exceptions_unnested ... ok
test_parallel_with_gil_continue_unnested (sequential_parallel)
Doctest: sequential_parallel.test_parallel_with_gil_continue_unnested ... ok
test_parallel_with_gil_return (sequential_parallel)
Doctest: sequential_parallel.test_parallel_with_gil_return ... ok
test_pointer_temps (sequential_parallel)
Doctest: sequential_parallel.test_pointer_temps ... ok
test_prange (sequential_parallel)
Doctest: sequential_parallel.test_prange ... ok
test_prange_break (sequential_parallel)
Doctest: sequential_parallel.test_prange_break ... ok
test_prange_continue (sequential_parallel)
Doctest: sequential_parallel.test_prange_continue ... ok
test_prange_in_with (sequential_parallel)
Doctest: sequential_parallel.test_prange_in_with ... ok
test_prange_matches_range (sequential_parallel)
Doctest: sequential_parallel.test_prange_matches_range ... ok
test_propagation (sequential_parallel)
Doctest: sequential_parallel.test_propagation ... ok
test_pure_mode (sequential_parallel)
Doctest: sequential_parallel.test_pure_mode ... ok
test_reassign_start_stop_step (sequential_parallel)
Doctest: sequential_parallel.test_reassign_start_stop_step ... ok
test_return (sequential_parallel)
Doctest: sequential_parallel.test_return ... ok
runTest (__main__.CythonCompileTestCase)
compiling (c/cy2) breaking_loop ... 
=== C/C++ compiler error output: ===
breaking_loop.c:1418:12: warning: ‘__pyx_f_13breaking_loop_func’ defined but not used [-Wunused-function]
 1418 | static int __pyx_f_13breaking_loop_func(CYTHON_UNUSED Py_ssize_t __pyx_v_n) {
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
====================================
ok
runTest (__main__.CythonCompileTestCase)
compiling (c/cy2) cimport_openmp ... ok
runTest (__main__.CythonCompileTestCase)
compiling (c/cy2) setup ... ok
runTest (__main__.CythonCompileTestCase)
compiling (c/cy2) simple_sum ... ok

======================================================================
FAIL: test_chunksize (parallel)
Doctest: parallel.test_chunksize
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.8/doctest.py", line 2204, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for parallel.test_chunksize
  File "/home/celelibi/code/cython/TEST_TMP/run/c/parallel/parallel.cpython-38-x86_64-linux-gnu.so", line unknown line number, in test_chunksize

----------------------------------------------------------------------
File "/home/celelibi/code/cython/TEST_TMP/run/c/parallel/parallel.cpython-38-x86_64-linux-gnu.so", line ?, in parallel.test_chunksize
Failed example:
    test_chunksize()
Expected:
    45
    45
    45
Got:
    45
    270
    360


======================================================================
FAIL: test_prange (parallel)
Doctest: parallel.test_prange
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.8/doctest.py", line 2204, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for parallel.test_prange
  File "/home/celelibi/code/cython/TEST_TMP/run/c/parallel/parallel.cpython-38-x86_64-linux-gnu.so", line unknown line number, in test_prange

----------------------------------------------------------------------
File "/home/celelibi/code/cython/TEST_TMP/run/c/parallel/parallel.cpython-38-x86_64-linux-gnu.so", line ?, in parallel.test_prange
Failed example:
    test_prange()
Expected:
    (9, 9, 45, 45)
Got:
    (9, 9, 360, 45)


----------------------------------------------------------------------
Ran 88 tests in 25.571s

FAILED (failures=2)
Times:
compile-c   :    19.37 sec  (  10,  1.937 / run) - slowest: 'c:parallel' (7.26s), 'c:sequential_parallel' (6.91s), 'c:parallel_swap_assign_T425' (2.02s), 'c:pure_parallel' (1.14s), 'c:numpy_parallel' (0.87s), 'c:setup' (0.32s), 'c:parallel_compile_float_rank' (0.23s), 'c:simple_sum' (0.21s)
cython      :     5.46 sec  (  11,  0.497 / run) - slowest: 'c:parallel' (1.72s), 'c:sequential_parallel' (1.64s), 'c:e_cython_parallel' (1.05s), 'c:parallel_compile_float_rank' (0.39s), 'c:numpy_parallel' (0.35s), 'c:parallel_swap_assign_T425' (0.17s), 'c:pure_parallel' (0.07s), 'c:cimport_openmp' (0.02s)
import      :     0.00 sec  (   5,  0.001 / run) - slowest: 'c:parallel' (0.00s), 'c:sequential_parallel' (0.00s), 'c:numpy_parallel' (0.00s), 'c:parallel_swap_assign_T425' (0.00s), 'c:pure_parallel' (0.00s)
pyimport    :     0.00 sec  (   1,  0.000 / run) - slowest: 'py:pure_parallel' (0.00s)
pyrun       :     0.00 sec  (   1,  0.001 / run) - slowest: 'py:pure_parallel' (0.00s)
run         :     0.33 sec  (   5,  0.066 / run) - slowest: 'c:parallel' (0.32s), 'c:sequential_parallel' (0.01s), 'c:numpy_parallel' (0.00s), 'c:parallel_swap_assign_T425' (0.00s), 'c:pure_parallel' (0.00s)
ALL DONE

The test failure doesn't depend on the C or C++ backend, doesn't depend on the compiler optimizations, doesn't depend on the version of python that runs runtest.py.

I know the tests passed on travis, but I would suggest not to dismiss this issue right away as it might be the symptom of a race condition.

@da-woods
Copy link
Contributor

Thanks for the output. I'm quite puzzled by it though. When I compare the .c file from tests/run/parallel.pyx (i.e. the two tests that are failing) with and without my PR the only differences are due to the extra test I added. If I revert the extra test I added then diff tells me that they're identical.

So I don't know I'm afraid... I guess the PR should be treated with caution because of what you say but I really don't understand where the differences could be coming from.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants