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

infill mode for main and server #3296

Merged
merged 9 commits into from
Oct 2, 2023
Merged

infill mode for main and server #3296

merged 9 commits into from
Oct 2, 2023

Conversation

vvhg1
Copy link
Contributor

@vvhg1 vvhg1 commented Sep 21, 2023

This adds support for the infill tasks in ./main and ./server.
There are still a few things that should be improved, but it works as is.
TODO:

  • update Main Readme
  • if context is too big in infill mode, we probably want to cut from both sides, need to make sure that neither prefix nor suffix get cut completely
  • entering new prompts in ./main is working but a bit awkward, user should be guided a bit more and prompting layout/feedback should be improved

PS hope it helps, thanks for the great work!!

@cebtenzzre cebtenzzre changed the title vvhg-code-infill infill mode for main and server Sep 21, 2023
@cebtenzzre cebtenzzre added the enhancement New feature or request label Sep 21, 2023
@ggerganov
Copy link
Owner

Should we make a separate infill example instead of adding to main?

@vvhg1
Copy link
Contributor Author

vvhg1 commented Sep 29, 2023

Should we make a separate infill example instead of adding to main?

It shares a lot with main but it might well make sense to not clutter the functionality of main too much.
I can have a look over the weekend. Shall I split the server infill part into a separate pr or do you want to keep both in the same pr?

@ggerganov
Copy link
Owner

I think a separate minimal infill example would be better.
The server changes can remain as they are.

@Green-Sky
Copy link
Collaborator

whats the difference between infill/this pr and fim/#2934 ?

@ggerganov
Copy link
Owner

I think they are very similar, but AFAIK #2934 does not work yet for some reason and I haven't had the time to check why. If we can make a similar example in this PR that works, then we can close #2934 and keep just the current one

@vvhg1
Copy link
Contributor Author

vvhg1 commented Sep 29, 2023

I think they are very similar, but AFAIK #2934 does not work yet for some reason and I haven't had the time to check why. If we can make a similar example in this PR that works, then we can close #2934 and keep just the current one

So more or less taking over the structure of #2934 and fill it with this example? Sounds good. What about the --interactive flag, is that valuable or do you just want the bare minimum? As it's already working with --interactive it shouldn't be any extra work to keep it.

vvhg1 and others added 3 commits September 30, 2023 13:47
* reverted changes to main and added infill example
@vvhg1
Copy link
Contributor Author

vvhg1 commented Sep 30, 2023

Ok, I'm done, no more changes in main.cpp, all moved to infill.cpp and added readme.
The conflict is easily resolved but for some reason I can't resolve it here, so I'd like to kindly ask you to do it on your end when merging.
Happy to work on it if you have further suggestions!

@cebtenzzre
Copy link
Collaborator

I tried to merge master into this PR, but your branch is protected so it won't let me:

remote: error: GH006: Protected branch update failed for refs/heads/master.
remote: error: Changes must be made through a pull request.
To github.com:vvhg1/llama.cpp.git
 ! [remote rejected]   pr-3296 -> master (protected branch hook declined)
error: failed to push some refs to 'github.com:vvhg1/llama.cpp.git'

@vvhg1
Copy link
Contributor Author

vvhg1 commented Sep 30, 2023

I tried to merge master into this PR, but your branch is protected so it won't let me:

remote: error: GH006: Protected branch update failed for refs/heads/master.
remote: error: Changes must be made through a pull request.
To github.com:vvhg1/llama.cpp.git
 ! [remote rejected]   pr-3296 -> master (protected branch hook declined)
error: failed to push some refs to 'github.com:vvhg1/llama.cpp.git'

could you try again, I unprotected my master

@vvhg1
Copy link
Contributor Author

vvhg1 commented Sep 30, 2023

Done, should all be working now.

@ggerganov ggerganov merged commit c97f01c into ggerganov:master Oct 2, 2023
32 checks passed
yusiwen pushed a commit to yusiwen/llama.cpp that referenced this pull request Oct 7, 2023
* vvhg-code-infill (ggerganov#1)

* infill in separate example (ggerganov#2)

* reverted changes to main and added infill example

* cleanup

* naming improvement

* make : add missing blank line

* fix missing semicolon

* brought infill up to current main code

* cleanup

---------

Co-authored-by: Cebtenzzre <cebtenzzre@gmail.com>
@amanda-peng
Copy link

amanda-peng commented Oct 10, 2023

Hi, is there any plan to implement non-stream mode for infill in server side?

@ggerganov
Copy link
Owner

Hi, is there any plan to implement non-stream mode for infill in server side?

Contributions are welcome

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

Successfully merging this pull request may close these issues.

5 participants