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.

Sign up for GitHub

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

Jump to bottom

feat: support deepseek-coder LLM #638

Merged
merged 6 commits into from
Jul 12, 2024
Merged

feat: support deepseek-coder LLM #638

merged 6 commits into from
Jul 12, 2024

Conversation

jcraftsman
Copy link
Contributor

@jcraftsman jcraftsman commented Jul 6, 2024 •
edited
Loading

Reference Issues/PRs

Fixes #637

What does this implement/fix?

In this PR, I implemented the changes suggested in issue #637, which involves adding support for DeepSeek Coder as a model option in SWE-agent. The following modifications were made:

  1. Model Integration: Added DeepSeek Coder to the list of selectable models in SWE-agent.
  2. I included two minor changes to enhance the development workflow:
  • Added a justfile: This file includes helpful commands for starting the server, frontend, and switching between DeepSeek and OpenAI APIs. (EDIT: I reverted back this change. justfile was not introduced in this PR)
  • Updated .gitignore: Simplified the switch between preconfigured keys*.cfg files by modifying the pattern to ignore all keys*.cfg files instead of just keys.cfg.

I tested these changes and it seems that DeepSeek Coder works seamlessly with existing SWE-agent functionalities.

I hope you find this small change useful. I'm available if you have any questions, and I really enjoy the work you’re doing with this project.

Thanks!

@ofirpress
Copy link
Member

Hi thanks for your contribution:

  1. Why do we need to make a new file for the deepseek keys? can't we just use the existing keys.cfg?
  2. also not sure what that justfile thing is for, maybe Kilian will get it

@jcraftsman
Copy link
Contributor Author

Hi @ofirpress ,

Thanks for your feedback!

  1. Initially, my changes made the environment variables OPENAI_API_KEY and OPENAI_API_BASE_URL applicable to both OpenAI and DeepSeek models, which required manual updates in keys.cfg when switching between them. I've now introduced DEEPSEEK_API_KEY and DEEPSEEK_API_BASE_URL for DeepSeek in the latest commit (8941058), eliminating the need for manual changes.
  2. I added a justfile to streamline my workflow (e.g., just start-server, just start-frontend) since ./start_web_ui.sh didn’t work for me. This change is isolated in commit ed9ec299 and can be easily removed if too specific.
    The updated .gitignore simplifies managing multiple preconfigured keys (e.g., keys.projectname.cfg).

I hope this clarifies my intent. Open to any further suggestions!

@jcraftsman
feat: add support deepseek-coder LLM: use separate prefix for environ…
65ec3ec 
…ment variables (DEEPSEEK_API_KEY and DEEPSEEK_API_BASE_URL)   configure "coder" as shortcut
@jcraftsman
build: ignore all keys*.cfg files (e.g. keys.pro.cfg, keys.personal.c…
8067a4b 
…fg, keys.project.cfg, keys.cfg won't be tracked)
@jcraftsman
Copy link
Contributor Author

I just dropped the justfile to avoid any confusion ^^

@klieret
Copy link
Member

klieret commented Jul 9, 2024

Thanks for opening the PR. I'll take a look at this in the next few days

klieret
klieret reviewed Jul 10, 2024
View reviewed changes
sweagent/agent/models.py Outdated Show resolved Hide resolved
@klieret
Copy link
Member

klieret commented Jul 10, 2024

Could you tick the "Allow edits from maintainers" checkbox:
image

I want to slightly refactor this

@jcraftsman
Copy link
Contributor Author

Hi @klieret,

I don't have access to the "Allow edits from maintainers" checkbox for this PR, likely because the changes are from a forked repo. However, I’ve invited you to the forked repo and granted full access. You can push your refactor to the support-deepseek-coder-LLM branch on umans-tech/SWE-crafter, and we'll see the changes in this PR.

Thanks!

@klieret klieret force-pushed the support-deepseek-coder-LLM branch from 2e4d05c to ff4bf5e Compare July 12, 2024 20:27
@klieret
Copy link
Member

klieret commented Jul 12, 2024

@jcraftsman Thanks! Could you check if it works with the last commit I've just pushed. After you give your OK, I'm happy to merge it :)

@jcraftsman
Copy link
Contributor Author

@klieret I really like your refactor, and everything continues to work perfectly. Thanks a lot! 🙂

@klieret klieret merged commit 5836582 into princeton-nlp:main Jul 12, 2024
4 checks passed
@klieret klieret deleted the support-deepseek-coder-LLM branch July 12, 2024 22:20
@klieret
Copy link
Member

klieret commented Jul 12, 2024

Thanks again for the PR! :)

klieret added a commit that referenced this pull request Sep 24, 2024
@jcraftsman @klieret
feat: support deepseek-coder LLM (#638)
88f1c2e 
* feat: support deepseek-coder LLM

* feat: add support deepseek-coder LLM: use separate prefix for environment variables (DEEPSEEK_API_KEY and DEEPSEEK_API_BASE_URL)   configure "coder" as shortcut

* build: ignore all keys*.cfg files (e.g. keys.pro.cfg, keys.personal.cfg, keys.project.cfg, keys.cfg won't be tracked)

* feat: support deepseek-coder LLM (let it fail when DEEPSEEK_API_BASE_URL is missing)

* Factor out DeepSeekModel

* Remove deepseek-coder from openai model shortcuts

---------

Co-authored-by: Kilian Lieret <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@klieret klieret klieret left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature Request: Add DeepSeek Coder LLM as a Model Option
3 participants
@jcraftsman @ofirpress @klieret