[RLlib] Smaller eval worker set fixes. #28811
Conversation
Signed-off-by: sven1977 <[email protected]>
sven1977
requested review from
gjoliver,
avnishn,
ArturNiederfahrenhorst,
smorad,
maxpumperla,
kouroshHakha and
krfricke
as code owners
| @@ -1147,6 +1148,11 @@ def remote_fn(worker, w_ref, w_seq_no): | |||
| # subsequent step results as latest evaluation result. | |||
| self.evaluation_metrics = {"evaluation": metrics} | |||
|
|
|||
| # Trigger `on_evaluate_end` callback. | |||
There was a problem hiding this comment.
This callback was missing for enable_async_evaluation=True setting.
rllib/algorithms/algorithm.py
Outdated
| @@ -2701,7 +2707,11 @@ def _run_one_evaluation( | |||
| if self.evaluation_workers is not None | |||
| else 0 | |||
| ) | |||
| eval_results["evaluation"]["num_recreated_workers"] = num_recreated | |||
| # Worker failures might have already been handled within `self._evaluate_async` | |||
There was a problem hiding this comment.
This count would be overridden by 0 for enable_async_evaluation=True.
There was a problem hiding this comment.
🤔 wonder if we should move this try_recover_from_step_attempt() into self.evaluate() then.
or, I actually need to refactor all the remote gets anyways for elastic training. so maybe I can clean this up when I get to remote_req_manager.
rllib/algorithms/algorithm.py
Outdated
| @@ -2701,7 +2707,11 @@ def _run_one_evaluation( | |||
| if self.evaluation_workers is not None | |||
| else 0 | |||
| ) | |||
| eval_results["evaluation"]["num_recreated_workers"] = num_recreated | |||
| # Worker failures might have already been handled within `self._evaluate_async` | |||
There was a problem hiding this comment.
🤔 wonder if we should move this try_recover_from_step_attempt() into self.evaluate() then.
or, I actually need to refactor all the remote gets anyways for elastic training. so maybe I can clean this up when I get to remote_req_manager.
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: sven1977 <[email protected]>
Signed-off-by: Weichen Xu <[email protected]>
Signed-off-by: sven1977 [email protected]
Smaller eval worker set fixes.
Why are these changes needed?
Related issue number
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.