Soft reboot Agents & Tools
Author: aymeric-roucher
Pull request: yes
Labels: name: Agents

# What does this PR do?

Following up on the discovery that Open Source models can be a competitive engine for agent setups (cf [this blog post](https://huggingface.co/blog/open-source-llms-as-agents)), this PR starts from our previous efforts on Agents to push further.

The driving idea of this change is to leave the user more freedom and clarity in creating agents: so we aim for small building blocks rather than monolithic classes, and will strive for simplicity.

We will enable sharing our Agents & Tools on the Hub, thus enabling efficient improvement by the community.

Main changes:
- Previous Agent classes `HfAgent`, `OpenaiAgent`, ... depended on the underlying LLM used as an engine. Although this ensured that the LLMs would be properly initialized, it introduced a black-box aspect that we remove here: we decide to resort to a simple `llm_callable` that the user has the responsability - thus the freedom - to initialize as they please.
- We keep basing the implementation on a `Tool` class and an `Agent` class, but propose two agents that the user can run: 
    - `CodeAgent` acts in one shot: generates all the necessary code to solve the task, then executes it all at once.
    - `ReactAgent` acts step by step: each step consists of one thought, then one function execution.

This is built with @Jofthomas and @cyril-k.

@LysandreJik @pacman100 the PR is still WIP: you're welcome to give your inputs! 🤗

For instance we still have the following questions:
- how do we constrain the `llm_callable` to raise informative errors if improperly initialized (for instance raise an error if it returns another type than a string) helping the user correct it, while still leaving them enough freedom to choose this llm as they want?
- which tests should we add: use remote tools ? (this might take more work since the `push_to_hub` method for tools seems broken)
- should we already add the support for tools running online (useful for instance for text-to-img tools) or should that be for a later PR?
- how can we unite more of the `CodeAgent` and `ReactAgent`'s `run` methods, to put as much of it as we can in the base `Agent` class?

Comments:
You can try out the different agent setups under `notebooks/agent.ipynb` (temporary file) to check how they work!

@LysandreJik some design questions:
- The previous `HfAgent` class used `requests` to query server for text generation: see [here](https://github.com/huggingface/transformers/blob/d45f47ab7f7c31991bb98a0302ded59ab6adac31/src/transformers/tools/agents.py#L595). We prefer to remove this part since we prefer to let the user define how they generate text (for instance with classes form `huggingface_hub`). But then the doc that refers to [HfAgent](https://github.com/huggingface/transformers/blob/main/docs/source/en/custom_tools.md) becomes unusable if we stay in transformers alone. Several solutions:
   - Replace the `HfAgent` in the doc with a `ReactAgent` using an `InferenceClient` from `huggingface_hub` for text generation (potential problem: showing an import from another package in transformers doc => risk of circular references?)
   - Remove this example in the doc altogether
   - Add the whole call to API done with `requests` in the doc, which is not concise at all
   
   Do you have a preferred option?

On Tools ([Original class](https://github.com/huggingface/transformers/blob/d45f47ab7f7c31991bb98a0302ded59ab6adac31/src/transformers/tools/base.py#L82), [Modified version of this PR](https://github.com/huggingface/transformers/blob/2493529cce6d3dd8bcfad3797996d83c61ff48bd/src/transformers/tools/base.py#L86)): 
- About the tool input definition: we think rather than a list which elements can be "text", "images", "audio", this should be a `Dict[str, type]`. With the type being anything: `str`, `float`, `bool`, images... For instance `{"expression": "2*4235", "keep_decimals": True}`.
    - The advantage of a dict vs a list is that it is not ordered, thus leaves the LLM a bit more freedom in changing the order of its function call, and each parameter has a name which should help the generation of proper arguments. 
    - Downside: these types are not gradio-native, which will force us to work a bit more when exporting a tool to a gradio element.
    - Point to discuss: **We can either use 1. `Dict[str, type]` or 2.`Dict[str, str]`** with the value as a string, for instance "str", "float", "bool". This is probably a matter of best practices. If we do option 1 we will have to define an enum of types maybe. Option 1 seems more robust and compatible with type checkers, while Option 2 lets more flexibility and is more serialization friendly IMO. **What do you think @LysandreJik ?**
cc @Jofthomas 

Thanks @aymeric-roucher. I don't have a problem with having `HfAgent` be reworked this way. It is written that the API is bound to evolve. If you want to have a `ReactAgent` that is more specific to react that's also fine by me (but I'd try to have most of the logic within `HfAgent`).

I don't have a direct preference regarding 1 vs 2 so let's see what you find the easiest to work with while working on your implementation and go from there? 

Also another design issue: what were the rationale for building this [very safe Python code interpreter](https://github.com/huggingface/transformers/blob/3a7789d3f6c089852d1cc6a387eae6c13b42f898/src/transformers/tools/python_interpreter.py) from the ground up @LysandreJik ? What are some redlines actions that we do not want the CodeAgent to do?

For now the code interpreter lacks many common operations like creating a dict or for loops. Even if we integrate more options we will not cover everything. So if we relax the safety requirements a bit we may envision to use a shorter and more versatile version like [this one](https://huggingface.co/spaces/lvwerra/python-interpreter/blob/main/python_interpreter_tool.py) (cc @lvwerra).

@LysandreJik this is ready for review! 🤗

The rationale for rebuilding the python code interpreter from scratch was that we wanted the Agent to only chain tools, and that's it. Anything else might have ended up a security risk so we wanted this to be very limited by design.

I'll take a look at the PR, thanks!

@LysandreJik after my 3 commits this afternoon, all the discussions we had should be solved on the code side: there's only the doc to do in a later PR!

PR Closed since it's merged in `agents` branch with https://github.com/huggingface/transformers/pull/29931.