[tune] Fix trial result fetching #4219
Conversation
| # the first available result, and we want to guarantee that slower | ||
| # trials (i.e. trials that run remotely) also get fairly reported. | ||
| # See https://github.com/ray-project/ray/issues/4211 for details. | ||
| [result_id], _ = ray.wait(shuffled_results) |
There was a problem hiding this comment.
You're probably right. The timeout was only relevant due to the backend bug. Since that is now fixed, the cause of the wait unfairness is likely just the ordering.
@robertnishihara I think one potential fix for this gotcha would be to return results in priority order by completion time, instead of order of ids passed in. I can see a lot of users running into this by accident.
| @@ -216,7 +217,13 @@ def get_running_trials(self): | |||
| return list(self._running.values()) | |||
|
|
|||
| def get_next_available_trial(self): | |||
| [result_id], _ = ray.wait(list(self._running)) | |||
| shuffled_results = random.sample( | |||
| self._running.keys(), len(self._running)) | |||
There was a problem hiding this comment.
nit: use random.shuffle() for clarity
|
This seems to solve my problem. Here's what the worker cpu usages look like with these changes: Unrelated to this issue, but the autoscaler start up seems extremely slow. It took >20 minutes to fully start up even though I defined |
|
Test FAILed. |
hartikainen
force-pushed
the
fix-trial-result-wait
branch
from
facdf64 to
f15ee40
Compare
hartikainen
force-pushed
the
fix-trial-result-wait
branch
from
f15ee40 to
a425faa
Compare
|
Test FAILed. |
|
Test FAILed. |
hartikainen
changed the title
hartikainen
changed the title
What do these changes do?
Fixes trial result fetching in
RayTrialExecutorby shuffling the results fetch objects.Related issue number
#4211