[TOD][Agents] Helper Agents for simulation + tests #4249
Conversation
Tests in next diff in stack.
…to simpler_tod_2a_teachers_only
| cand[tod.STANDARD_API_NAME_SLOT] | ||
| == call[tod.STANDARD_API_NAME_SLOT] | ||
| ): | ||
| schema = cand |
There was a problem hiding this comment.
just for my understanding... this selects the last API call made as the (singular) dialogue goal?
There was a problem hiding this comment.
Nope, for k dialogue goals in a dialogue, there are now k dialogues generated each with a single goal..
This code says that if there are multiple schemas listed with the same API call name (which ideally shouldn't happen though I don't have validation for this active), it'll grab the last of those to use as the schema.
parlai/core/tod/tod_agents.py
Outdated
| """ | ||
|
|
||
| def __init__(self, opt: Opt, shared=None): | ||
| # This class represents two "agents" so need to make sure we don't increment episode number (reset) twice |
There was a problem hiding this comment.
Worth mentioning the beginning part of this comment into the class heading comment above for TodApiCallAndSysUttAgent. Additionally clarifying which two agents <-- on this latter point I wasn't sure as this seems to be just the System agent? Are you counting it as two because it alternates between API calls and NLG response to the user?
There was a problem hiding this comment.
Updated comment here to say.
# This class will have `act()` called on it twice per round — once for API call and once for NLG — so need to make sure we don't increment episode number (reset) prematurely; use the `already_reset` flag for this. ```
parlai/core/tod/tod_agents.py
Outdated
| "text": tod.STANDARD_API_SCHEMAS, | ||
| "id": self.id, | ||
| "domain": self.episode.domain, | ||
| "episode_down": False, |
There was a problem hiding this comment.
episode_done ? (not down)
There was a problem hiding this comment.
Well that's a weird typo lol.
parlai/core/tod/tod_agents.py
Outdated
| self.already_reset = False | ||
| if tod.STANDARD_API_SCHEMAS in self.observation.get("text", ""): | ||
| return { | ||
| "text": tod.STANDARD_API_SCHEMAS, |
There was a problem hiding this comment.
This returns something like "APIS: " and nothing more, i.e. any empty schema. Is that correct? I guess it is if this is only used for offline model testing under the assumption that the schema is unknown to the model at production time... but maybe worth adding a comment that mentions the expected use case of this agent (offline model testing) in the class heading.
There was a problem hiding this comment.
Ahhhhh this is a convention I put in for the first turn. Can leave it as a comment. (Implicitly assumes that we're comparing against NO SCHEMA models, but that's what we use for evaluation.)
parlai/core/tod/tod_agents.py
Outdated
| } | ||
| self.round_idx += 1 | ||
|
|
||
| self.api_call_turn ^= True |
There was a problem hiding this comment.
I think I was being paranoid that I would forget to set that variable to the opposite of itself at one point.. will address.
| f"USER: user_utt_{episode_idx}_0", | ||
| "APICALL: ", | ||
| "APIRESP: ", | ||
| f"SYSTEM: sys_utt_{episode_idx}_0", |
There was a problem hiding this comment.
why not use the tod_core constants for USER, APICALL, etc?
There was a problem hiding this comment.
Oof good minor catch.
Effectively does same thing as #4176 but broken out to be easier to review.
This diff does 2 things:
tod_agents.py.This is the same as #4232 but I pressed the wrong button there.