On Thu, Jul 1, 2021, 2:54 PM Gabe Joseph ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In docs/source/scheduling-state.rst <#5018 (comment)>: > @@ -310,5 +310,6 @@ API .. autoclass:: Scheduler :members: + :inherited-members: I added this to get SchedulerState.decide_worker, but maybe we don't want all the Server, etc. methods showing up, and I should add decide_worker explicitly? ------------------------------ In docs/source/scheduling-policies.rst <#5018 (comment)>: > + a b c d + \ \ / / + X + +In the above case, we want ``a`` and ``b`` to run on the same worker, +and ``c`` and ``d`` to run on the same worker, reducing future +data transfer. We can also ignore the location of ``X``, because assuming +we split the ``a b c d`` group across all workers to maximize parallelism, +then ``X`` will eventually get transferred everywhere. +(Note that wanting to co-locate ``a b`` and ``c d`` would still apply even if +``X`` didn't exist.) + +Calculating these cousin tasks directly by traversing the graph would be expensive. +Instead, we use the task's TaskGroup, which is the collection of all tasks with the +same key prefix. (``(random-a1b2c3, 0)``, ``(random-a1b2c3, 1)``, ``(random-a1b2c3, 2)`` +would all belong to the TaskGroup ``random-a1b2c3``.) I wish we could describe the heuristic without explaining TaskGroups (or at least that they were documented elsewhere), but I couldn't figure out how without something verbose and awkward. @mrocklin <https://github.com/mrocklin> in general I recognize that this addition is probably too much detail on the implementation, and could be much shorter and just describe the scheduler's objective of co-assigning related tasks. I'm not sure if we want to go into the details of the implementation here or not. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5018 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACKZTHDQLHZQ7YZLFA5OKDTVTPXPANCNFSM47VQ2BLQ> .