[TOD][Agents] Helper Agents for simulation + tests #4232
Conversation
This reverts commit 85f1c52.
|
|
||
| Implements an "epoch done" | ||
|
|
||
| Member variables assumed to be set in init downstream: |
There was a problem hiding this comment.
just wondering if you could use 'ABC' and include an abstract method that downstream is forced to implement
There was a problem hiding this comment.
Thinking about this more, this comment doesn't exactly make any sense. I'll deal with it... in the __init__ here...
| This is not actually "episode_done" so much as "we want to signify to the world | ||
| that we have gone past the batch". | ||
|
|
||
| This class should not control whether or not the episode is actually done since |
There was a problem hiding this comment.
Confused as to what the aim is here?
There was a problem hiding this comment.
Ah this is per the conversation structure — It's assumed by convention that the User agent ends the conversation with a [DONE] token... can make this more clear.
There was a problem hiding this comment.
Edit: After staring at this a bit more, realized it's clearer just editing the code to be more explicit rather than trying to explain what's going on.
| return episode_idx % 2 == 1 and max_turns > 0 | ||
|
|
||
|
|
||
| def use_broken_api_calls_this_turn(round_idx, episode_idx): |
There was a problem hiding this comment.
"used_" as in already happened and testing that it happened, or "includes_" as in this round includes a broken API call?
Possibly reading this wrong but I don't think this is a signal to "use" something?
There was a problem hiding this comment.
This file generates mock conversations. This is an arbitrary function to decide to use a broken API call once in a while in a deterministic way to test if the #s in the metrics is correct.
I can make this comment more clear.
There was a problem hiding this comment.
Seeing this in context again, I can make it more clear relative to the other function above.
| return [ | ||
| tod_core.TodStructuredRound( | ||
| user_utt=f"user_utt_{episode_idx}_{round_idx}", | ||
| api_call_machine=make_api_call_machine( |
There was a problem hiding this comment.
wondering if there might be a better postfix, other than "_machine", which expresses a list of dicts structure?
There was a problem hiding this comment.
Discussed offline; we can use _dict rather than _machine here (but will do in a diff at the end of the stack)
Effectively does same thing as #4176 but broken out to be easier to review.
This diff does 2 things:
tod_agents.py.