-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: enabled saving and evaluation for moderator (#271) #272
base: demo
Are you sure you want to change the base?
Conversation
* feat: enable saving AgentProfile and chat history on redis * fix: AgentProfile can now be loaded by EpisodeLog correctly * feat: enable evaluation of EpisodeLog * [autofix.ci] apply automated fixes * fix: use EpisodeLog and AgentProfile from sotopia directly --------- Co-authored-by: JXZhou <JXZhou> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
if len(self.message_history) == 1: | ||
self.message_history[0].append( | ||
self.message_history.append( | ||
[ | ||
( | ||
agent_action.agent_name, | ||
"Environment", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JXZhou0224 let's think this a bit more carefully, a messge should be (sender, receivers (seperated by comma), message content)
, so maybe we should not put environment here anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Certiainly! I will fix it. I set the receiver to "Enivronment" because in the render_for_human()
method in EpisodeLog
requires the message to be sent to "Environment" in order to render successfully. I think I will change it to "All" to represent sending to everyone and rewrite the render_for_human()
method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see, maybe that's fine if environment defaults to all.
last_turn=self.scenario, | ||
last_turn=json.dumps( | ||
{ | ||
"use_pk_value": self.use_pk_value, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hyperparameter that determines "whether to use AgentProfile from exisiting database" is set in the moderator. Therefore, moderator will communicate this information to all Agents before the start of conversation.
epilog.save() | ||
# print(epilog.render_for_humans()) | ||
if self.push_to_db: | ||
epilog.save() | ||
return epilog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we need to change this to evaluation queue or something
self.model_name: str = model_name | ||
self.agent_profile_pk: str = agent_pk | ||
self.name: str = agent_name | ||
self.background: dict = background |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note self.background
is not used anywhere when generating the agent actions
feat: enable saving AgentProfile and chat history on redis
fix: AgentProfile can now be loaded by EpisodeLog correctly
feat: enable evaluation of EpisodeLog
[autofix.ci] apply automated fixes
fix: use EpisodeLog and AgentProfile from sotopia directly
Closes #
📑 Description
✅ Checks
type/descript
(e.g.feature/add-llm-agents
)ℹ Additional Information