[RLlib] - Add env and agent steps in custom evaluation function for comformity with metrics logger.

[simonsays1980]

Why are these changes needed?

Algorithm.evaluate() evaluates a custom evaluation function in the new stack, but then adds env_steps=agent_steps=0 to the NUM_ENV_STEPS_SAMPLED_LIFETIME, NUM_AGENT_STEPS_SAMPLES_LIFETIME b/c all other evaluation methods return env_steps and agent_steps together with results.

This PR modifies the return type of the custom evalaution fucntion to metrics, env_steps, agent_steps in the new api stack (i.e. enable_env_runner_and_connector_v2=True).

Related issue number

Closes #44595

Checks

• I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
• I've run scripts/format.sh to lint the changes in this PR.
• I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've added any new APIs to the API Reference. For example, if I added a
    method in Tune, I've added it in doc/source/tune/api/ under the
    corresponding .rst file.
• I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
• Testing Strategy
  • Unit tests
  • Release tests
  • This PR is not tested :(

[sven1977]

Remove from this PR. I think it belongs to another PR?

[simonsays1980]

What exactly do you mean? The comment was there before.

[sven1977]

nit: Can we give the user the exact expected return signature here?

Tuple[ResultDict, int, int]?

[sven1977]

Look good to me. Thanks @simonsays1980 for the fix.

Some thoughts on the entire problem (10k feet view):
In the mid-run, we should probably change to a world, in which we don't return anything anymore from any "core" method and instead, just log stuff inside these methods to the metrics logger:

• algo.training_step() -> Algo should call metrics.reduce() later and not visible to the user
• Learner.update() -> Already done and hidden from user
• Learner.additional_update() -> method should probably disappear entirely
• EnvRunner.sample/get_metrics -> should probably be combined and metrics.reduce call should be automated under the hood (hidden from user).

This way, the user would never have to deal with when to call reduce and would simply log their metrics, plain and simple.

Also, we need to unify our timesteps structures:

env_steps_lifetime: [some count]
agent_steps_lifetime/
    [agent 1]: [some count]
    [agent 2]: [some count]
module_steps_lifetime/
     [module 1]: [some count]

[simonsays1980]

Look good to me. Thanks @simonsays1980 for the fix.

Some thoughts on the entire problem (10k feet view): In the mid-run, we should probably change to a world, in which we don't return anything anymore from any "core" method and instead, just log stuff inside these methods to the metrics logger:

• algo.training_step() -> Algo should call metrics.reduce() later and not visible to the user
• Learner.update() -> Already done and hidden from user
• Learner.additional_update() -> method should probably disappear entirely
• EnvRunner.sample/get_metrics -> should probably be combined and metrics.reduce call should be automated under the hood (hidden from user).

This way, the user would never have to deal with when to call reduce and would simply log their metrics, plain and simple.

Also, we need to unify our timesteps structures:

env_steps_lifetime: [some count]
agent_steps_lifetime/
    [agent 1]: [some count]
    [agent 2]: [some count]
module_steps_lifetime/
     [module 1]: [some count]

Hiding the reduce call and taking responsibility out of a user's control would be a great step towards more user-friendliness. The sensitivity of where to call reduce makes this call complicated for the user and not dealing with it anymore makes would make things easier.