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

[FIX] LegacyMultiProc hangs up indefinitely #2773

Closed

Conversation

oesteban
Copy link
Contributor

@oesteban oesteban commented Nov 9, 2018

This PR relates to #2700, and should fix the problem
underlying #2548.

I first considered adding a control thread that monitors
the Pool of workers, but that would require a large overhead
keeping track of PIDs and polling very often.

Just adding the core file of bpo-22393
should fix #2548

Summary

Fixes # .

List of changes proposed in this PR (pull-request)

Acknowledgment

  • (Mandatory) I acknowledge that this contribution will be available under the Apache 2 license.

This PR relates to nipy#2700, and should fix the problem
underlying nipy#2548.

I first considered adding a control thread that monitors
the `Pool` of workers, but that would require a large overhead
keeping track of PIDs and polling very often.

Just adding the core file of [bpo-22393](python/cpython#10441)
should fix nipy#2548
@oesteban
Copy link
Contributor Author

oesteban commented Nov 9, 2018

(I haven't tested this yet, though)

@effigies
Copy link
Member

As the Python 3.8 version of this, I'm not sure how well this will play with 2.7 and 3.4, at the least.

@oesteban
Copy link
Contributor Author

Yep, unfortunately, the fix is python +3.7. I'd say we start recommending switching to python 3.7 if the workflow is memory-hungry.

@effigies
Copy link
Member

Given the recent numpy bug, I'm hesitant to suggest moving scientific applications to 3.7 until upstream projects have had a bit more time to find and fix critical bugs.

How hard would this be to port to 3.6? I suspect if we get it working there, 3.5 and 3.4 will fall into place quickly.

I'm okay with abandoning 2.7, as long as we don't break current usage further.

@oesteban
Copy link
Contributor Author

Some thoughts:

  • As per your link, anaconda fixed the bug in numpy 1.15.2, and it is fixed with 1.15.3 with Pypi. I guess we should update the pinned versions if python == 3.7.
  • I'm not proposing to discontinue python <3.7, I would just make this fix available for python +3.7.
  • If nothing of the above works, then I'd say it should not be too hard to backport to 3.6 or even 3.5 (prior to 3.5 it would be harder). Although I'm not sure it is worthwhile.

@codecov-io
Copy link

codecov-io commented Nov 12, 2018

Codecov Report

Merging #2773 into master will decrease coverage by 3.4%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2773      +/-   ##
=========================================
- Coverage    67.5%   64.1%   -3.41%     
=========================================
  Files         340     338       -2     
  Lines       43271   43223      -48     
  Branches     5364    5362       -2     
=========================================
- Hits        29210   27707    -1503     
- Misses      13360   14446    +1086     
- Partials      701    1070     +369
Flag Coverage Δ
#smoketests ?
#unittests 64.1% <100%> (-0.82%) ⬇️
Impacted Files Coverage Δ
nipype/pipeline/plugins/legacymultiproc.py 65.51% <100%> (+0.51%) ⬆️
nipype/interfaces/nilearn.py 40% <0%> (-56.67%) ⬇️
nipype/utils/spm_docs.py 25.92% <0%> (-44.45%) ⬇️
nipype/interfaces/freesurfer/base.py 49.59% <0%> (-30.9%) ⬇️
nipype/utils/logger.py 59.7% <0%> (-29.86%) ⬇️
nipype/algorithms/rapidart.py 35.39% <0%> (-29.21%) ⬇️
nipype/interfaces/spm/base.py 58.41% <0%> (-29.05%) ⬇️
nipype/utils/provenance.py 55.73% <0%> (-28.99%) ⬇️
nipype/interfaces/fsl/model.py 55.26% <0%> (-25.35%) ⬇️
nipype/testing/fixtures.py 77.33% <0%> (-21.34%) ⬇️
... and 42 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 199d9a6...b220c86. Read the comment docs.

@oesteban oesteban changed the title [FIX] LegacyMultiProc hangs up indefenitely [FIX] LegacyMultiProc hangs up indefinitely Nov 13, 2018
@oesteban oesteban closed this Nov 13, 2018
@oesteban oesteban deleted the fix/legacy-multiproc-broken-pool branch November 13, 2018 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MultiProc goes into infinite loop (resource management related)
3 participants