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

Report remaining budget to user #3313

Closed
wants to merge 27 commits into from

Conversation

rob-luke
Copy link
Contributor

@rob-luke rob-luke commented Apr 26, 2023

Background

Closes #6
Following #762, this PR reports the remaining and total budget to the user.

Changes

Reports the budget to the user if the budget is set.
The budget is report before "NEXT ACTIONS"

Documentation

Modified print_assistant_thoughts to report the remaining budget and the total budget.

Test Plan

Wrote tests. But you can also see the change in the figure below

image

Questions to reviewers

  • Is this too noisy? Would you prefer I make this only show if the user passes in a flag like python -m autogpt --report_budget?
  • I find it useful to also include the total budget. But if this is too noisy I can remove it
  • There were circular import issues. So the import is done in the function. Alternatively, I could make larger changes to the codebase, but this seemed the simplest change to make. Let me know if you want me to take another path.

PR Quality Checklist

  • My pull request is atomic and focuses on a single change.
  • I have thoroughly tested my changes with multiple different prompts.
  • I have considered potential risks and mitigations for my changes.
  • I have documented my changes clearly and comprehensively.
  • I have not snuck in any "extra" small tweaks changes

@vercel
Copy link

vercel bot commented Apr 26, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 8, 2023 1:00am

@codecov
Copy link

codecov bot commented Apr 26, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.29 ⚠️

Comparison is base (dc95959) 61.31% compared to head (61b09e8) 61.03%.

❗ Current head 61b09e8 differs from pull request most recent head ebbeb48. Consider uploading reports for the commit ebbeb48 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3313      +/-   ##
==========================================
- Coverage   61.31%   61.03%   -0.29%     
==========================================
  Files          72       72              
  Lines        3304     3313       +9     
  Branches      542      543       +1     
==========================================
- Hits         2026     2022       -4     
- Misses       1141     1152      +11     
- Partials      137      139       +2     
Impacted Files Coverage Δ
autogpt/logs.py 84.02% <100.00%> (+0.89%) ⬆️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rob-luke rob-luke marked this pull request as draft April 26, 2023 12:00
@github-actions github-actions bot added size/l and removed size/m labels Apr 26, 2023
@rob-luke rob-luke marked this pull request as ready for review April 26, 2023 12:21
@rob-luke
Copy link
Contributor Author

FYI @Vwing

@Pwuts Pwuts added the API costs Related to monitoring/reduction of running costs label Apr 27, 2023
@vercel vercel bot temporarily deployed to Preview April 27, 2023 21:04 Inactive
@Pwuts Pwuts self-assigned this Apr 27, 2023
@Pwuts Pwuts removed their assignment Apr 27, 2023
@Pwuts
Copy link
Member

Pwuts commented Apr 27, 2023

Tests fail @rob-luke

@rob-luke
Copy link
Contributor Author

rob-luke commented Apr 28, 2023

Tests fail @rob-luke

The tests started failing when you merged main. But I have fixed the issues. So this should be good for review @Pwuts @collijk @BillSchumacher

@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Apr 28, 2023
@github-actions
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Apr 28, 2023
@github-actions
Copy link
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

@vercel vercel bot temporarily deployed to Preview May 3, 2023 01:48 Inactive
Copy link
Contributor

@Vwing Vwing left a comment

Choose a reason for hiding this comment

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

I tested it out, and it looks great! Thank you for taking this on, @rob-luke.

Copy link
Contributor

@Vwing Vwing left a comment

Choose a reason for hiding this comment

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

Oops, I forgot to press "viewed" 😅

@vercel vercel bot temporarily deployed to Preview May 4, 2023 04:46 Inactive
@vercel vercel bot temporarily deployed to Preview May 4, 2023 18:18 Inactive
@rob-luke
Copy link
Contributor Author

rob-luke commented May 4, 2023

Thanks @Vwing and @merwanehamadi
I’m keeping this updated with main. Is there anyone I should ping for another review/merge?

@Boostrix Boostrix mentioned this pull request May 4, 2023
1 task
@rob-luke
Copy link
Contributor Author

rob-luke commented May 4, 2023

The failing test is unrelated to this change. I do not have access to restart the CI. When there is a push to main I will merge it in and that will restart the CI.

@p-i-
Copy link
Contributor

p-i- commented May 5, 2023

This is a mass message from the AutoGPT core team.
Our apologies for the ongoing delay in processing PRs.
This is because we are re-architecting the AutoGPT core!

For more details (and for infor on joining our Discord), please refer to:
https://github.com/Significant-Gravitas/Auto-GPT/wiki/Architecting

@vercel vercel bot temporarily deployed to Preview May 5, 2023 03:10 Inactive
@vercel vercel bot temporarily deployed to Preview May 5, 2023 22:50 Inactive
@rob-luke
Copy link
Contributor Author

rob-luke commented May 5, 2023

Thanks for the info @p-i-
I read the wiki and suggest this Pr comes under point 1 and should be merged before reintegration

@Boostrix
Copy link
Contributor

Boostrix commented May 6, 2023

I [...] suggest this Pr comes under point 1 and should be merged before reintegration

I agree. This is a rather simple change but such a useful one. And while it's not as extensive as #3466, it solves the problem for most people for now. It may need to be revisited soonish to come up with a more generic implementation - but for now it's definitely candidate material to be considered for a review despite the ongoing re-arch effort.

Also, it's worth keeping in mind that this change is perfectly in line with all project recommendations to monitor API usage. So, definitely do consider this for integration now:

https://github.com/Significant-Gravitas/Auto-GPT#readme

Please note that the use of the GPT-4 language model can be expensive due to its token usage. By utilizing this project, you acknowledge that you are responsible for monitoring and managing your own token usage and the associated costs. It is highly recommended to check your OpenAI API usage regularly and set up any necessary limits or alerts to prevent unexpected charges.

@vercel vercel bot temporarily deployed to Preview May 6, 2023 16:57 Inactive
@Boostrix
Copy link
Contributor

Boostrix commented May 7, 2023

it seems, there's a dedicated budget manager implementation in progress: #3919

@vercel vercel bot temporarily deployed to Preview May 8, 2023 01:00 Inactive
@rob-luke
Copy link
Contributor Author

rob-luke commented May 8, 2023

Thanks for the info @Boostrix
In that case I will close this PR. Thanks anyway @Vwing

@rob-luke rob-luke closed this May 8, 2023
@Boostrix
Copy link
Contributor

Boostrix commented May 8, 2023

Would not close it yet. Your patch looks straightforward and should work for many folks "as is" while the other stuff still is in the pipeline and might take a couple of weeks before it really arrives in the form of a functional feature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API costs Related to monitoring/reduction of running costs size/l
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Make Auto-GPT aware of it's running cost
7 participants