search

caporaso-lab / sourcetracker2

SourceTracker2
BSD 3-Clause "New" or "Revised" License
60 stars 45 forks source link

ENH: Replaces ipyparallel with multithreading #117

Closed johnchase closed 5 years ago

johnchase commented 5 years ago

fixes #106, fixes #98, fixes #92 This pull request replaces ipyparallel with the python multithreading library. This should prevent some of the issues that we have seen running sourcetracker in parallel previously

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.4%) to 99.612% when pulling eb1a91c420ad49f80452c130b0a6909a7add6a2e on issue-106 into b81220f26ca566cd1f131cc7f17c826320a96076 on master.

wdwvt1 commented 5 years ago

@johnchase - I'll take a look at this over the next 2 days!

johnchase commented 5 years ago

Thanks @jakereps I updated the travis settings so that it will show build status on the prs (although, now it's kicking off two pipelines, I'll look into that). I pulled out the Pool code so it's only in one place, and added a test with two processors. I did not update tests to pass with python > 3.5 as I think that may be a bigger discussion

wdwvt1 commented 5 years ago

Do you want to squash and merge or should we discuss the pyhton 3.7 changes that would be necessary?

johnchase commented 5 years ago

Thanks @wdwvt1!

should we discuss the pyhton 3.7 changes that would be necessary?

I think this should be a separate issue, although likely a worthwhile

jkliu commented 5 years ago

Branch works fine for me, except when I define jobs too high. README.md still mentions ipyparallel.

johnchase commented 5 years ago

Branch works fine for me, except when I define jobs too high.

@jkliu Can you be specific about what doesn't work, and what too many jobs are?

jkliu commented 5 years ago

When I define 581 jobs, SourceTracker doesn't finish in a couple hours. I would also get some sort of "file save" error from jupyter hub (didn't take a screenshot).

wdwvt1 commented 5 years ago

@johnchase thanks for all the work, I merged this branch and added the issue #119 to deal with the too many jobs issue raised by @jkliu.