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

Replace test_(do_not_)steal_communication_heavy_tasks tests with more robust versions #7243

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions distributed/tests/test_steal.py
Original file line number Diff line number Diff line change
Expand Up @@ -1446,6 +1446,57 @@ def func(*args):
assert (ntasks_per_worker < ideal * 1.5).all(), (ideal, ntasks_per_worker)


def test_balance_steal_communication_heavy_tasks():
dependencies = {"a": 10, "b": 10}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I change this line to {"a": 1e-6, "b": 1e-6} the test remains green, and if go lower than that I instead get "ValueError: Expected a value larger than 16 integer but got 10."
Are we sure we're actually testing anything here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test breaks if we double the cost of the dependencies to {"a": 20, "b": 20} as the tasks have become too expensive to move in this setup. By reducing the cost of the dependencies, we are checking whether we would move tasks that are cheap to move which is the desired behavior. Reducing the cost below 1e-6 creates trouble since we are calculating the actual size of the dependencies as the product of their specified cost and the available bandwidth.

dependency_placement = [["a"], ["b"]]
task_placement = [[["a", "b"]] * 10, []]

def _correct_placement(actual):
actual_task_counts = [len(placed) for placed in actual]
return sum(actual_task_counts) == 10 and actual_task_counts[1] > 0

_run_dependency_balance_test(
dependencies,
dependency_placement,
task_placement,
_correct_placement,
)


def test_balance_uneven_without_replica():
dependencies = {"a": 1}
dependency_placement = [["a"], []]
task_placement = [[["a"], ["a"]], []]

def _correct_placement(actual):
actual_task_counts = [len(placed) for placed in actual]
return actual_task_counts == [2, 0]

_run_dependency_balance_test(
dependencies,
dependency_placement,
task_placement,
_correct_placement,
)


def test_balance_eventually_steals_large_dependency_without_replica():
dependencies = {"a": 10}
dependency_placement = [["a"], []]
task_placement = [[["a"]] * 20, []]

def _correct_placement(actual):
actual_task_counts = [len(placed) for placed in actual]
return sum(actual_task_counts) == 20 and actual_task_counts[1] > 0

_run_dependency_balance_test(
dependencies,
dependency_placement,
task_placement,
_correct_placement,
)


def test_balance_even_with_replica():
dependencies = {"a": 1}
dependency_placement = [["a"], ["a"]]
Expand Down