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

Feat set token limits based on model #4498

Conversation

BaseInfinity
Copy link
Contributor

Background

Every day we see people come into the discord explaining how they ran into a token limit because most people are still on a 3.0/3.5 key. The maintainer's built the app to assume 8000 token limit due to gpt4 key and atm this just causes friction because in reality, most people do not have a 4.0 key

To meet in the middle we will respect the user's token limit but compare it to the max token limit based off the smart/fast llvm model and set a limit 15% less to ensure we take into account the prompt and package

Changes

Use the model's max_token_limits for both fast/smart llvm

Test Plan

i updated failing tests

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

BaseInfinity and others added 28 commits May 11, 2023 16:25
…its-based-on-model' into feat--set-token-limits-based-on-model
@vercel
Copy link

vercel bot commented May 31, 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 31, 2023 10:15pm

@vercel vercel bot temporarily deployed to Preview May 31, 2023 22:15 Inactive
@Auto-GPT-Bot
Copy link
Contributor

You changed AutoGPT's behaviour. The cassettes have been updated and will be merged to the submodule when this Pull Request gets merged.

@codecov
Copy link

codecov bot commented May 31, 2023

Codecov Report

Patch coverage: 66.66% and project coverage change: -0.04 ⚠️

Comparison is base (dae58f8) 69.70% compared to head (dd987f7) 69.67%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4498      +/-   ##
==========================================
- Coverage   69.70%   69.67%   -0.04%     
==========================================
  Files          72       72              
  Lines        3562     3558       -4     
  Branches      569      569              
==========================================
- Hits         2483     2479       -4     
  Misses        890      890              
  Partials      189      189              
Impacted Files Coverage Δ
autogpt/config/config.py 74.48% <ø> (-1.02%) ⬇️
autogpt/agent/agent.py 59.88% <66.66%> (+0.48%) ⬆️

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

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

github-actions bot commented Jun 6, 2023

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

@BaseInfinity
Copy link
Contributor Author

Not sure what to do with this pull request, essentially waiting to see if we should allow users to set the token limits still or always infer them by model

@Pwuts / @Boostrix / @ntindle / @k-boikov

I also understand this is probably low priority but figured I'd keep the conversation going =)

@Boostrix
Copy link
Contributor

Boostrix commented Jun 6, 2023

Low priority is relative, it's one of the more frequent issues on discord and github

https://discord.com/channels/1092243196446249134/1092243196798582930/1115737448580907180

Pwuts
Pwuts previously approved these changes Jun 6, 2023
Copy link
Member

@Pwuts Pwuts left a comment

Choose a reason for hiding this comment

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

Looks good to me :)

@Pwuts
Copy link
Member

Pwuts commented Jun 6, 2023

Conflict is because test_config.py has been moved in master

@Pwuts Pwuts added this to the v0.4.1 Release milestone Jun 6, 2023
@erik-megarad
Copy link
Contributor

Hell yeah, nice one

@vercel
Copy link

vercel bot commented Jun 7, 2023

Deployment failed with the following error:

Resource is limited - try again in 6 hours (more than 100, code: "api-deployments-free-per-day").

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

github-actions bot commented Jun 7, 2023

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

@github-actions github-actions bot added size/m and removed size/l labels Jun 7, 2023
@Pwuts Pwuts self-assigned this Jun 7, 2023
@Pwuts Pwuts merged commit 1e851ba into Significant-Gravitas:master Jun 7, 2023
@Boostrix
Copy link
Contributor

Boostrix commented Jun 7, 2023

We may still want to support changing this at runtime so that the agent can exit gracefully using a corresponding error message, in order for the LLM to change some variables at runtime: https://discord.com/channels/1092243196446249134/1092423060923101304/1115963736642035732

@BaseInfinity
Copy link
Contributor Author

Thanks @Pwuts for getting this merged! I went away for a couple days and was pleasantly surprised to see this merged lol

@Pwuts Pwuts mentioned this pull request Jun 13, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants