[rllib] [rfc] Proposed class renames

[ericl]

Per discussion with several users, there are some possible renames we could do to clarify the internal architecture:

• Rename rllib.evaluation.PolicyGraph to rllib.policy.Policy. Similarly, rllib.evaluation.TFPolicyGraph becomes rllib.policy.TFPolicy and so on. The files are moved to a new rllib/policy dir.

• Move the rllib/agents directory to rllib/train.

• Add TF qualifier for consistency with Torch policies
  A3CPolicyGraph => A3CTFPolicy
A3CTorchPolicyGraph => A3CTorchPolicy

• Rename PolicyEvaluator to RolloutWorker
  deprecate compute_gradients() / apply_gradients()

• Consolidate [local_evaluator, remote_evaluators] to single WorkerSet object

  workers = WorkerSet(...)
  local_evaluators => workers.local_worker()
  remote_evaluators => workers.remote_workers()
  

For the most part, I think we can do these renames without breaking backwards compatibility by leaving aliases behind. Though, I'm not sure how easy moving an entire directory is.

[ericl]

cc @hartikainen @bbalaji-ucsd

[bbalaji-ucsd]

+1 for all changes, except agents

Move the rllib/agents directory to rllib/train

I'm conflicted about this one given that agents is used in Coach, TF Agents, RLGraph, ChainerRL. I like the point you raised in one of the threads that this is confusing when multi-agents are used. I quite like the use of algos in Spinning Up.

There are also two concepts here:

1. The algorithm that puts together various aspects of learning a policy - loss, model, replay buffer sampling, exploration, filters, etc.
2. The orchestrator that manages things like initialize policy, interact with env, collect samples, update policy, evaluate and so on.
   Which of these two is train? The verb aligns well with the latter in my opinion, and perhaps can be algo agnostic.

[ericl]

We could leave it as agents, since this would just be the status quo. Algos does seem slightly nicer, but maybe not nice enough to do a rename.

The algorithm that puts together various aspects of learning a policy - loss, model, replay buffer sampling, exploration, filters, etc.
The orchestrator that manages things like initialize policy, interact with env, collect samples, update policy, evaluate and so on.
Which of these two is train? The verb aligns well with the latter in my opinion, and perhaps can be algo agnostic.

Here's the current division of these concepts:

loss, model, exploration -> Policy (rllib.policy)

sampling, filters -> RolloutWorker (rllib.evaluation) (arguably, filters should be made into a trainable part of the policy, though this would require some work to implement)

replay, sgd -> PolicyOptimizer (rllib.optimizers) (or perhaps TrainingStrategy?)

putting it all together -> Trainer (rllib.agents)

[richardliaw]

I think PolicyOptimizer can stay as is; I've never found it too confusing (and TrainingStrategy reminds me of "DistributedStrategy" which isn't too informative.

[bbalaji-ucsd]

This is where there is a lack of consensus in implementations across libraries.

loss, model, exploration -> Policy (rllib.policy)

Most libraries call this the agent.

putting it all together -> Trainer (rllib.agents)

Coach calls this graph_manager. I like the name trainer. I can't find equivalent in TF Agents.

Fine with sticking to PolicyOptimizers.

Just to point out why things are inconsistent and confusing: TF Agents refers to exploration as policies, ChainerRL refers to optimizers for rmsprop_async and policies for action distribution.

[hartikainen]

Generally I really like the idea of clarifying the class/module names.

Rename rllib.evaluation.PolicyGraph to rllib.policy.Policy.
This makes sense. Unrelated to the naming though, I think it's a little confusing that currently all the rl logic is incorporated in the policy graphs, meaning that the PolicyGraph includes things like value functions etc., which in my opinion are separate from the policies, and the interaction (e.g. training policy using the value functions) should be handled by the Algorithm or Trainer. I.e., currently, the Rllib seems to be organized as something like the following:

• Trainer
  • PolicyGraph
    • Policies pi(a|s)
    • Value functions V(s), Q(s,a)
  • Environment
  • PolicyEvaluator (or more generally EnvironmentSampler)
  • Replay Pool

Whereas I think it's more natural to have the value functions etc. on the same level with the Policy:

• Trainer
  • Policies pi(a|s)
  • Value functions V(s), Q(s,a)
  • Reward functions r(s,a,s')
  • Whatever else is needed to learn by the algorithm
  • Environment
  • PolicyEvaluator (or more generally EnvironmentSampler)
  • Replay Pool

Move the rllib/agents directory to rllib/train.

At some point I thought that train/trainer would be a good name for these. On the hindsight, though, I actually think that Algorithm might be a better name, mainly because these classes seem to correspond exactly to what we generally call a reinforcement algorithm, e.g. SAC/PPO/DDPG/whatever. I don't feel strongly though.

Add TF qualifier for consistency with Torch policies
A3CPolicyGraph => A3CTFPolicy
A3CTorchPolicyGraph => A3CTorchPolicy

👍

Rename PolicyEvaluator to RolloutWorker

RolloutWorker sounds good to me. Could also be something like EnvironmentSampler?

deprecate compute_gradients() / apply_gradients()

Can you elaborate on this?

Consolidate [local_evaluator, remote_evaluators] to single WorkerSet object

👍

[ericl]

Policies pi(a|s)
Value functions V(s), Q(s,a)
Reward functions r(s,a,s')
Whatever else is needed to learn by the algorithm

While conceptually there isn't a reason to combine them from the RL point of view, it's valuable to group them into one opaque object that "the system" can manipulate. Otherwise, things like weight synchronization, multi-agent, etc. become harder to manage, especially in a distributed setting.

However that doesn't mean we can't internally decompose Policy into different orthogonal concepts that can be easily put together. That should be easy to implement incrementally. Note that custom models already have stuff like value_function().

RolloutWorker sounds good to me. Could also be something like EnvironmentSampler?

Yeah my only requirement here is it ends with *Worker, to connect with the num_workers config. I'm partial to EvaluationWorker since it seems like a nice clarification and isn't that much of a change from the current naming.

deprecate compute_gradients() / apply_gradients()

Originally having these two functions available on PolicyEvaluator and PolicyGraph was important for asynchronous optimization (i.e., A3C). However it turns out asynchronous optimization isn't that great when async sampling will do, so it makes sense to deprecate these functions.

[bbalaji-ucsd]

I'd prefer RolloutWorker over EvaluationWorker. In my mind, EvaluationWorker refers to evaluating the policy without exploration. OpenAI Five uses similar terminology:

[bbalaji-ucsd]

In the Agent class, we can have a train() function, so that the Trainer abstraction is maintained.

[ericl]

Ok, I agree RolloutWorker is less ambiguous. No intent to rename anything to agent though, we just went through getting rid of the term agent in the last release :)