YosefLab / Cassiopeia

A Package for Cas9-Enabled Single Cell Lineage Tracing Tree Reconstruction
https://cassiopeia-lineage.readthedocs.io/en/latest/
MIT License
75 stars 24 forks source link

Single thread hybrid solver #233

Closed colganwi closed 7 months ago

colganwi commented 10 months ago

Currently, cas.solver.HybridSolver cannot be run in parallel because daemonic processes cannot spawn new processes. This PR solves this by modifying cas.solver.HybridSolver to not use multiprocessing when threads = 1.

codecov[bot] commented 10 months ago

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

:exclamation: No coverage uploaded for pull request base (master@5b8a452). Click here to learn what that means.

:exclamation: Current head bcae813 differs from pull request most recent head 42cd03f. Consider uploading reports for the commit 42cd03f to get more accurate results

Files Patch % Lines
cassiopeia/solver/ILPSolver.py 50.00% 5 Missing :warning:
cassiopeia/solver/HybridSolver.py 80.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #233 +/- ## ========================================= Coverage ? 79.50% ========================================= Files ? 89 Lines ? 8072 Branches ? 0 ========================================= Hits ? 6418 Misses ? 1654 Partials ? 0 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

colganwi commented 7 months ago

This is for the case of running the hybrid solver in parallel with multiprocessing. Since multiprocessing creates demonic threads these threads cannot then call multiprocessing again to create more threads.

The logging changes enable better control of the logging behavior since the logging output become quite long when hybrid solver is called in parallel. Specifically, logfile can be set to None to disable logging and the default stdout logging level has been changed to WARNING. If you still want the hybrid solver to print to stdout you can override the logging level with:

logging.basicConfig(level=logging.INFO)

mattjones315 commented 7 months ago

Hi @colganwi,

Great, that makes a lot more sense now. I've seen the same issue pop up, so thanks for the PR. Looks great to me and will merge it in.