Skip to content
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

Running agent.arun(input=df) with environment=None is totally valid. #98

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chemeris
Copy link

Otherwise this code requires us to create an dummy async environment which is not used in the code and not even possible with the current code base.

Otherwise this code requires us to create an dummy async environment
which is not used in the code and not even possible with the current
code base.
@robot-ci-heartex robot-ci-heartex marked this pull request as draft April 24, 2024 08:06
Copy link
Contributor

@niklub niklub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. Although I totally agree with your proposal, it is still interesting how do you specifically use async call without async environment? It would be great if you could provide an example code so we would incorporate it in our future tutorials

@chemeris
Copy link
Author

chemeris commented Apr 25, 2024

Thank you, and I look forward to having this merged.

I can't provide an example code, unfortunately, but the code we're writing is based on the classification_skill.ipynb example. We simply changed the synchronous agent.run() to asynchronous await agent.arun().

Two reasons for this:

  1. We're running in an async FastAPI handler anyway, so why not?
  2. The async agent supports setting temperature=0, while sync one doesn't. And given the default temperature is AFAIK set to 1, it reduces reliability of the classification. And in general it looked like sync code is pretty much abandoned in favor for the new async one.

That said, I always thought that an environment is only needed during "teaching". Are you saying there are other use cases for it during normal production operation? The documentation is not very detailed on this part.

@chemeris
Copy link
Author

Hi, is there anything else required to merge this PR?

@chemeris chemeris marked this pull request as ready for review May 5, 2024 16:45
@chemeris
Copy link
Author

chemeris commented May 5, 2024

@niklub Please, could you merge this PR?
I see that the change is approved but it's still says "This workflow requires approval from a maintainer"

@robot-ci-heartex robot-ci-heartex marked this pull request as draft May 6, 2024 03:05
@chemeris
Copy link
Author

Any news?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants