Design questions from porting Acme and SB3 into AOS

[nickjalbert]

This is a centralized spot to record notes and questions that fall out of our work in porting Acme and SB3 agents into AOS.

[nickjalbert]

Some collected feedback from @andyk after he took a look at the Acme port:

• What should the directory that contains AOS backing data be called (currently .acr/, probably makes sense to rename .aos/)?
• Should AOS backing data be stored in a dot directory (e.g. .acr/ or .aos/) or a regular directory (e.g. acr/ or aos/)?
• The system that AOS uses to communicate data location is hacky (i.e. decrating functions with a data_location member). Is there an easier way?
• Should the Agent class actually be named Coordinator?
• Should the agent.ini contain hierarchical info? Some components rely on other components and it doesn't make sense to have one installed and not the other. It also might be a way to type components, check interfaces, etc.
• Is an agent directory an "agent" or an "agent project"?
• Where should the functions like rollout() and step() live? They are currently in the Agent base class, but you could make a case that it should be a part of the runtime (i.e. hidden and well integrated into the programming model) or in the agent.py file that gets created when you init an agent (i.e. straightforwardly customizable and explicit).

[andyk]

Is an agent directory an "agent" or an "agent project"?

A little more context on this: currently, agentos creates (and looks for) a file called agent.ini which is a type of agent manifest. I.e., it declares which components should be used for agentos operations (e.g., agentos run, agentos learn, agentos install, agentos list, etc.). SEPARATELY, one of the core AgentOS base classes is agentos.Agent and ACS expects agent.ini to specify a component of type "Agent" is (others are Policy, Environment, etc.).

So in our current naming scheme/usage, the concept of "Agent" refers to 3 different concepts:

1. An ACS component type and the associated core AgentOS base class (agentos.Agent). These are used together to create an ACS agent component that glues together other sub-components (e.g., a policy, trainer, experience dataset)
2. the manifest file (agent.ini) used to compose components
3. the directory in which the manifest file lives

One of my other ideas that Nick captured could address this. namely...

Should the Agent class actually be named Coordinator?

My favorite solution right now is to rename agent.ini to agentos.ini and think of the directory it lives in not as an "agent", but as an "AgentOS project" and then leave the high level glue component type as agentos.Agent (i.e., don't rename it to agentos.Coordinator).

[andyk]

A new observation:

I see you wrote a new _run() method inside cli.py (here) that the CLI uses now instead of core.run_agent(). We may want to keep the main runtime logic inside core.py and keep the logic inside of cli.py minimal so that we can easily have a programmatic API to the AgentOS runtime that is parallel to the CLI, as depicted in this architecture slide:

Also, I know you've been in quick and dirty prototyping mode, but even in that mode it might be better to either evolve an existing function or completely replace it instead of leaving it around as a zombie :-P

[andyk]

Eventually, it could make make sense to break out the runtime logic into a separate file from the core abstractions, something like runtime.py. Then we could move much of the logic that is currently in cli.py into it. For example, install, run_agent, load_agent_from_path, etc. Then cli.py can be minimal and contain only calls to the runtime functions plus any other logic specific to the CLI interface.

[nickjalbert]

Yeah, this makes sense. I think I was hesitant to kill or massively restructure functions like core.run_agent is because I was hoping to

1. avoid breaking the example agents, and
2. avoid having to port all the example agents to a new API

Especially before we all agreed on the new direction. However, we're probably far enough along that it makes sense to do some cleanup.

I also agree it's a good idea to keep the CLI as thin as possible and expose a set of runtime functions that can be used programmatically.

[andyk]

Observation: the word "iterations" (currently a flag to agentos run) is a bit ambiguous; depending on the context, it could mean num_episodes or num_steps (within an episode). Might want to make the flag -e/--episodes instead.

[andyk]

It looks like you've added a new arg actions to agentos.core.Policy.decide() (here).

Compare this to the version in 0.1.0 that does not pass actions:

agentos/agentos/core.py

Line 64 in 288e868

def decide(self, observation):

Based on the behavior of the template policy (here), I think the purpose of that is to pass in the action space, and then the decide() function can pick one of the actions from it (e.g., via random.choice() which takes a sequence).

1. If this parameter is necessary, should we call it action_space?
2. The template policy won't work for continuous action spaces, right?

[andyk]

Just noticed that in your current version of agentos.cli.learn() (here) you call agent.learn() but you haven't added that to the agentos.Agent base class as an abstract method yet.