Made it easier to instantiate environments #74
Conversation
dosubot
bot
added
size:M
src/aviary/env.py
Outdated
| @classmethod | ||
| def from_name(cls, name: str, task: str | None = None, **env_kwargs) -> Self: | ||
| new_cls = _get_cls_from_name(ENV_REGISTRY, name) | ||
| if task is not None: | ||
| return new_cls.from_task(task) | ||
| return new_cls(**env_kwargs) |
There was a problem hiding this comment.
Does this become a problem if a subclass uses task in its init?
There was a problem hiding this comment.
Will add a check for that
| self.end_immediately = end_immediately | ||
| self.task = task |
There was a problem hiding this comment.
Can we name this self.subject = task?
There was a problem hiding this comment.
Would rather just keep it consistent - don't think it's so important here about what is done with the task but like to be able to trace its path.
| self.end_immediately = end_immediately | ||
| self.task = task |
| @@ -432,7 +469,7 @@ def __bool__(self) -> bool: | |||
| return True | |||
|
|
|||
|
|
|||
| def _construct_obj_from_name(registry: dict[str, tuple[str, str]], name: str, **kwargs): | |||
| def _get_cls_from_name(registry: dict[str, tuple[str, str]], name: str): | |||
There was a problem hiding this comment.
Nice change here, like it
| @@ -248,8 +248,35 @@ async def close(self) -> None: | |||
| """ | |||
|
|
|||
| @classmethod | |||
| def from_name(cls, name: str, **env_kwargs) -> Self: | |||
| return _construct_obj_from_name(ENV_REGISTRY, name, **env_kwargs) | |||
| def from_task(cls, task: str) -> Self: | |||
There was a problem hiding this comment.
Not sure I understand the use-case for from_task. Is there a specific environment where this kind of behavior is desirable?
There was a problem hiding this comment.
I understand is for inference time?
There was a problem hiding this comment.
Presumably task is not an arbitrary string as a user prompt would be? It seems as though it must correspond to a valid problem_id?
There was a problem hiding this comment.
This is for inference time - so that you can have user defined tasks instead of tasks coming from a training or eval set.
There was a problem hiding this comment.
An example would help I think
There was a problem hiding this comment.
This is to enable scripts/entry points so that an end user can use the environments
There was a problem hiding this comment.
Left a comment above, it makes the most sense to me for environments like HotpotQA where question can be open-ended. I don't know what happens, however, when the user passes in an arbitrary problem_id to environments like GSM8K? Similarly in cloning, I can't see where self.problem_id is used?
There was a problem hiding this comment.
I missed that the problem argument is also being set as task
| @@ -50,6 +50,10 @@ def __init__( | |||
| self.check_tool = Tool.from_function(self.check_answer) | |||
| self.tools = [self.calc_tool, self.check_tool] | |||
|
|
|||
| @classmethod | |||
| def from_task(cls, task: str) -> "CalculatorEnv": | |||
| return cls(problem_id="task", problem=task, answer=0.0) | |||
There was a problem hiding this comment.
Where is self.problem_id used in the GSM8K environment aside from being exported as a dictionary in export_frame?
There was a problem hiding this comment.
I don't know - it's a required argument so I just put a placeholder. Do you think I should refactor to make id optional?
There was a problem hiding this comment.
Ah, I missed that task is being passed in to problem
| @@ -191,6 +191,10 @@ def __init__( | |||
| create_tool(self.finish, "Finish"), | |||
| ] | |||
|
|
|||
| @classmethod | |||
| def from_task(cls, task: str) -> "HotPotQAEnv": | |||
| return cls(question=task, correct_answer=0.0) | |||
There was a problem hiding this comment.
I think HotpotQA makes the most sense intuitively for me since question can be open-ended allowing the user to pass an arbitrary question at inference time.
| in calling an LLM. This is how the environment should be used after training | ||
| and in deployment. We don't take config here, because the default environment config | ||
| should be general for arbitrary tasks. Or, the config should be coupled to the agent | ||
| training (future TODO). |
There was a problem hiding this comment.
Might be worth adding some examples in the docstring e.g.
- For the HotpotQA environment, a question not featured in the HotpotQA dataset.
- For the GSM8K environment, a math word question not featured in the GSM8K dataset.
There was a problem hiding this comment.
Ok - will add these in next revision set
Added a new way to create environments based on an explicit task. Also made it possible to see available environments (although it is approximate).