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

Reconfigure dask futures and client management #958

Merged
merged 4 commits into from
Sep 20, 2022

Conversation

nfahlgren
Copy link
Member

@nfahlgren nfahlgren commented Sep 19, 2022

Describe your changes
This PR simplifies the pcv.parallel.multiprocess function to use the Dask client map function to submit jobs to the cluster. Map creates a futures object that can then be monitored with progress. The aim here is to properly populate the client cluster and shut it down upon completion to eliminate client errors that have been oberved.

Type of update
Is this a: update

Associated issues
#922

For the reviewer
See this page for instructions on how to review the pull request.

  • PR functionality reviewed in a Jupyter Notebook
  • All tests pass
  • Test coverage remains 100%
  • Documentation tested
  • New documentation pages added to plantcv/mkdocs.yml
  • Changes to function input/output signatures added to updating.md
  • Code reviewed
  • PR approved

@nfahlgren nfahlgren added work in progress Mark work in progress update Updates an existing feature/method labels Sep 19, 2022
@nfahlgren nfahlgren added this to the PlantCV v4.0 milestone Sep 19, 2022
@nfahlgren nfahlgren added this to Pull Requests in PlantCV4 via automation Sep 19, 2022
@codecov
Copy link

codecov bot commented Sep 19, 2022

Codecov Report

Merging #958 (f77786d) into release-4.0 (457f072) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##           release-4.0      #958   +/-   ##
=============================================
  Coverage       100.00%   100.00%           
=============================================
  Files              161       161           
  Lines             6860      6857    -3     
=============================================
- Hits              6860      6857    -3     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
plantcv/parallel/multiprocess.py 100.00% <100.00%> (ø)

@nfahlgren nfahlgren added ready to review and removed work in progress Mark work in progress labels Sep 19, 2022
@nfahlgren
Copy link
Member Author

I have tested this using LocalCluster on my computer and HTCondorCluster on the cluster and it seems to avoid the client errors we observed sometimes before - but still functions the same

@nfahlgren nfahlgren merged commit 44e12ce into release-4.0 Sep 20, 2022
PlantCV4 automation moved this from Pull Requests to Done Sep 20, 2022
@nfahlgren nfahlgren deleted the update-dask-client-job-submission branch September 20, 2022 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to review update Updates an existing feature/method
Projects
PlantCV4
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants