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

Refactored prompt.py to reduce token usage #1996

Merged
merged 29 commits into from
Jun 9, 2024

Conversation

temotskipa
Copy link
Contributor

Refactored prompt.py to reduce token usage. Also included a band-aid fix to resolve an issue Devin encountered while editing prompt.py. The issue in question was that random example commands included in prompt.py were getting executed in the terminal, which resulted in the agent getting stuck in a loop trying to edit the file unsuccessfully. This issue should be resolved in a timely manner with a proper fix imo.

@enyst enyst requested a review from xingyaoww May 23, 2024 11:23
Copy link
Collaborator

@enyst enyst left a comment

Choose a reason for hiding this comment

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

Can you please tell, on what version did you encounter getting stuck in a loop? There is a fix merged on main just these days for that kind of issue, although I suspect it may still be missing commands because of different command ids. Do you by any chance have logs from that issue?

IMHO some edits are a great idea, but I hope @xingyaoww can take a look when we consider this kind of changes.

@temotskipa
Copy link
Contributor Author

I encountered it on the opendevin:main docker image.

Copy link
Collaborator

@yufansong yufansong left a comment

Choose a reason for hiding this comment

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

I prefe to not change the prompts only if we can do experiments to show it do works.

@neubig
Copy link
Contributor

neubig commented May 23, 2024

I agree with @yufansong , this seems like a great change if it doesn't reduce our accuracy, but I am worried that it might. Maybe we could run swe bench lite with this

rbren
rbren previously requested changes May 23, 2024
Copy link
Collaborator

@rbren rbren left a comment

Choose a reason for hiding this comment

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

CC @xingyaoww

Most likely a lot of these extra tokens are required for quality--the LLM often needs some reminding.

But there are also a few typo fixes etc here so we might want to take some of it.

@xingyaoww
Copy link
Member

xingyaoww commented May 23, 2024

Yep! I agree with @yufansong @neubig @rbren -- These typo changes are actually good. How about we wait until #1941 is merged (since it also changes a bunch of prompt and bump the version to v1.5). Then we merge all the typo fixes here -- so we can just do ONE pass eval of SWE-Bench lite to make sure we are not degrading performance?

@temotskipa
Copy link
Contributor Author

temotskipa commented May 23, 2024

Is there anything else that I can do, or do I just wait for #1941 to be merged?

@xingyaoww
Copy link
Member

@temotskipa I think that PR is merged :) feel free to re-adjust the prompt!

@temotskipa
Copy link
Contributor Author

I mean I personally think this prompt is fine, but IDK if the maintainers also think so.

@xingyaoww
Copy link
Member

@temotskipa Can you resolve those merge conflict so we can review again and try to merge it?

@rbren
Copy link
Collaborator

rbren commented Jun 3, 2024

@temotskipa are you interested in pushing this one forward? @xingyaoww do you have specific changes you'd like to see?

@yufansong
Copy link
Collaborator

@temotskipa are you interested in pushing this one forward? @xingyaoww do you have specific changes you'd like to see?

I suggest to hold any prompts change before we finishing all benchmark evaluation.

@xingyaoww
Copy link
Member

xingyaoww commented Jun 3, 2024

I am mainly looking for changes that fix the conflicts - once those are fixed, I'm happy with merging this PR (after 2 days - when we finish running all the eval for the paper as yufan suggested)

@temotskipa
Copy link
Contributor Author

So am I supposed to revert the changes and just fix any typos and things like that? It's unclear to me what is conflicting here.

@xingyaoww
Copy link
Member

@temotskipa Sorry for getting back late! We were running for a huge deadline :(
Feel free to keep whatever you want to change! As long as the conflict is gone, we can review and merge it :)

@neubig neubig assigned xingyaoww and unassigned temotskipa Jun 8, 2024
@enyst enyst mentioned this pull request Jun 9, 2024
yueqis and others added 9 commits June 9, 2024 13:04
* Add files via upload

* Update README.md

* Update run_infer.py

* Update utils.py

* make lint

* Update evaluation/toolqa/run_infer.py

---------

Co-authored-by: Engel Nyst <enyst@users.noreply.github.com>
Co-authored-by: yufansong <yufan@risingwave-labs.com>
Co-authored-by: Boxuan Li <liboxuan@connect.hku.hk>
* revert change in file action

* remove useless code

* make lint
* feat: add gpqa benchmark evaluation

* add metrics

* reset configs in final block

* make lint

---------

Co-authored-by: yufansong <yufan@risingwave-labs.com>
…nDevin#2329)

* remove bottom chatbox fade

* Modal wider; fix lint error

* settings: attempt to not clear api key for same provider

* prevent api key from resetting after changing the model

* revert other changes and fix post test tear down error

---------

Co-authored-by: amanape <83104063+amanape@users.noreply.github.com>
…uck OpenDevin#1895] (OpenDevin#2034)

* fix: codeact bug OpenDevin#1895

* fix: add CmdRunAction timeout hint.

* Update agenthub/codeact_agent/prompt.py

Co-authored-by: Engel Nyst <enyst@users.noreply.github.com>

* regenerate integration test

---------

Co-authored-by: Engel Nyst <enyst@users.noreply.github.com>
Co-authored-by: Graham Neubig <neubig@gmail.com>
Co-authored-by: yufansong <yufan@risingwave-labs.com>
* removed unused files from gorilla

* Update run_infer.py, removed unused imports

* Update utils.py

* Update ast_eval_hf.py

* Update ast_eval_tf.py

* Update ast_eval_th.py

* Create README.md

* Update run_infer.py

* make lint

* Update run_infer.py

* fix lint

---------

Co-authored-by: yufansong <yufan@risingwave-labs.com>
@xingyaoww
Copy link
Member

Integration test should be fixed as well -- @rbren @enyst feel free to take a look and unblock the PR if possible :)

@li-boxuan li-boxuan requested a review from rbren June 9, 2024 17:18
@li-boxuan li-boxuan dismissed rbren’s stale review June 9, 2024 17:18

I have a PR #2326 which also tweaks the prompt a bit. Follow-ups could be included in that PR.

@li-boxuan li-boxuan merged commit e925cef into OpenDevin:main Jun 9, 2024
18 checks passed
@temotskipa temotskipa deleted the concise-prompts branch June 9, 2024 19:22
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