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

vine: dask arg worker_transfers #3943

Merged

Conversation

JinZhou5042
Copy link
Member

@JinZhou5042 JinZhou5042 commented Sep 27, 2024

Proposed Changes

While calling DaskVine get API, we have two arguments for the same purpose, worker_transfers and lazy_transfers, the later one is marked as deprecated.

The default value for both is True. If the users want to activate it, they may pass

  • worker_transfers=True
  • lazy_transfers=True
  • worker_transfers=True, lazy_transfers=True

This works well. However, if they want to deactivate it, they may pass

  • worker_transfers=False
  • lazy_transfers=False
  • worker_transfers=False, lazy_transfers=False

Only the last works, because worker_transfers or lazy_transfers is always True if only one argument is explicitly passed by the users.

This PR changes the logic to - if one of the two arguments is False, then self.worker_transfers is False, otherwise True.

Merge Checklist

The following items must be completed before PRs can be merge.
Check these off to verify you have completed all steps.

  • make test Run local tests prior to pushing.
  • make format Format source code to comply with lint policies. Note that some lint errors can only be resolved manually (e.g., Python)
  • make lint Run lint on source code prior to pushing.
  • Manual Update Update the manual to reflect user-visible changes.
  • Type Labels Select a github label for the type: bugfix, enhancement, etc.
  • Product Labels Select a github label for the product: TaskVine, Makeflow, etc.
  • PR RTM Mark your PR as ready to merge.

@JinZhou5042 JinZhou5042 self-assigned this Sep 27, 2024
@JinZhou5042 JinZhou5042 added bug For modifications that fix a flaw in the code. TaskVine labels Sep 27, 2024
@JinZhou5042 JinZhou5042 requested a review from btovar September 27, 2024 20:19
@btovar btovar merged commit 78e0276 into cooperative-computing-lab:master Oct 24, 2024
8 checks passed
btovar pushed a commit that referenced this pull request Nov 6, 2024
* vine: dask arg worker_transfers

* typo

* typo
@JinZhou5042 JinZhou5042 deleted the dask_worker_transfers branch November 18, 2024 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For modifications that fix a flaw in the code. TaskVine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants