Skip to content
This repository was archived by the owner on Jun 24, 2024. It is now read-only.

Structural Overhaul#162

Merged
philpax merged 40 commits intorustformers:mainfrom
danforbes:dfo/models/gpt2
Apr 30, 2023
Merged

Structural Overhaul#162
philpax merged 40 commits intorustformers:mainfrom
danforbes:dfo/models/gpt2

Conversation

@danforbes
Copy link
Copy Markdown
Contributor

Replaces #141. Includes code from #85 and #125.

  • GGML is now a submodule and a single crate
  • Support for GPT-2 architecture
  • Structural refactor into llm, llm-base, model, and llm-cli crates

@philpax
Copy link
Copy Markdown
Collaborator

philpax commented Apr 30, 2023

Aha, I was just about to reply to the BLOOM PR!

I would suggest keeping BLOOM in - there's no reason to remove that code if someone can fix it with some work (we'll make an issue for it) - but we'll remove it from the list of features, and make a note on the crate itself.

As for the rest, let me have a look...

Comment thread README.md

<!-- markdownlint-disable-file MD026 -->
This project is a Rust port of
[llama.cpp](https://github.com/ggerganov/llama.cpp) 🦙🦀🚀
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm thinking we'll remove this wording as we grow to accommodate more LLMs. I'll revise the wording on this after this PR lands, so nothing for you to do here - just mentioning it.

@danforbes
Copy link
Copy Markdown
Contributor Author

I don't see the value of keeping non-working code in the main branch. I would suggest keeping BLOOM in a feature branch until it's working.

Comment thread README.md Outdated
Comment thread ggml-rs/Cargo.toml Outdated
Comment thread ggml-rs/build.rs Outdated
@hhamud
Copy link
Copy Markdown
Contributor

hhamud commented Apr 30, 2023

btw could you add me as a git commit author rather than a mention as you did when you upstreamed the ggml changes. @danforbes

@philpax
Copy link
Copy Markdown
Collaborator

philpax commented Apr 30, 2023

I don't see the value of keeping non-working code in the main branch. I would suggest keeping BLOOM in a feature branch until it's working.

Code that isn't kept in the main branch tends to rot, especially if we start making larger changes. As far as I know, BLOOM is done, we just need to figure out what's causing it to fail generation.

It's much easier for someone to pick it up and fix it if it's present; I think it's also a good way to ensure our abstractions work as expected as we increase the number of models.


Apart from that, PR looks great! Well done :) Looking forward to merging this, I think it's a huge improvement.

@hhamud
Copy link
Copy Markdown
Contributor

hhamud commented Apr 30, 2023

I don't see the value of keeping non-working code in the main branch. I would suggest keeping BLOOM in a feature branch until it's working.

Code that isn't kept in the main branch tends to rot, especially if we start making larger changes. As far as I know, BLOOM is done, we just need to figure out what's causing it to fail generation.

It's much easier for someone to pick it up and fix it if it's present; I think it's also a good way to ensure our abstractions work as expected as we increase the number of models.

Apart from that, PR looks great! Well done :) Looking forward to merging this, I think it's a huge improvement.

I personally think we should just merge and place a comment on the issue of it not working. I'd pick it up again but I don't have time for it till next week.

danforbes and others added 2 commits April 30, 2023 08:14
Co-authored-by: @hhamud <53880692+hhamud@users.noreply.github.com>
@danforbes
Copy link
Copy Markdown
Contributor Author

I've added back the BLOOM crate and tagged @hhamud as a co-author ✅

@philpax
Copy link
Copy Markdown
Collaborator

philpax commented Apr 30, 2023

All that's left are the formatting and restoring bindings generation. Are you OK for me to do that, or do you have changes in the works?

@danforbes
Copy link
Copy Markdown
Contributor Author

Please go for it - thank you so much 👯

@philpax philpax merged commit 7d650ac into rustformers:main Apr 30, 2023
@danforbes danforbes deleted the dfo/models/gpt2 branch April 30, 2023 21:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants