Combine `generate()` functions #1675

apaz-cli · 2024-08-14T16:23:46Z

Combining the implementations for litgpt.chat.base.generate() and litgpt.chat.base.generate(). Both are used everywhere, and have very different assumptions, so keeping both around, wrapping the original.

WIP, commented out nonsense, tests broken, wanted to get it out there so you could look over it and see if I'm on the right track.

The HF token is a throwaway made specifically for this purpose because I want to see how CI will react, and if it will OOM or not. Will not be in the final version, but I want to make sure it works with llama3 and I can't seem to replicate the test environment.

tests/test_chat.py

apaz-cli · 2024-08-15T05:32:41Z

I'm calling it a night, but the PR is pretty much done. All that's left is to fix the new test_litgpt_chat_endtoend() and test_litgpt_generate_endtoend() so that they load a model from the checkpoint dir already on the device. And to rewrite test_decode(), to test decode_stream() because I ripped out decode().

The excruciating pain of dealing with stop_tokens in a way that's sane cannot be overstated. I bashed my head into it for hours. But now it works, and all of our models are better supported. We should also see a nice TTFT increase, since we now longer have to hold on to buffer_length tokens before we start yielding them. This is a genuine triumph, I feel.

removed hf token

Co-authored-by: Sebastian Raschka <mail@sebastianraschka.com>

apaz-cli · 2024-08-15T06:08:11Z

I just did an interactive rebase to edit my first commit to take the HF token out of it. Hilariously though, Github retains it anyway. Thanks Github.

I wonder what people are supposed to do when they leak credentials that are actually important.

rasbt

Looks like a good restructure at first glance. Just some early comments. There'll be more!

tests/test_chat.py

rasbt · 2024-08-15T20:49:53Z

litgpt/tokenizer.py

+            # TODO: Is there a way to not have to do this?
+            # This may actually affect our tokens per second.
+
+            # sentencepiece does not support decoding token-by-token because it adds spaces based on the surrounding tokens


@Andrei-Aksionov reimplemented the tokenizer pipeline and may have ideas here

Interesting. I think I didn't test the ~~hack~~ fix in decode method (with a dummy_token_id) for SentencePiece tokenizer.
So maybe now the logic below is not needed.

…/litgpt into ap/combine_generage

This reverts commit a4fc1c8, reversing changes made to 28454b3.

rasbt · 2024-08-21T14:35:10Z

@apaz-cli Yes, long story about that. I will describe that to you offline

litgpt/generate/base.py

Co-authored-by: Sebastian Raschka <mail@sebastianraschka.com>

tests/test_api.py

litgpt/api.py

README.md

rasbt · 2024-08-22T15:39:26Z

litgpt/api.py

-                )
-            else:
-                total_devices = use_devices
+        num_devices = calculate_number_of_devices(devices)


Any reason why this was changed? Bad rebase maybe?

Yeah. Not sure when it happened.

Co-authored-by: Sebastian Raschka <mail@sebastianraschka.com>

litgpt/generate/base.py

rasbt reviewed Aug 14, 2024

View reviewed changes

tests/test_chat.py Outdated Show resolved Hide resolved

tests/test_chat.py Outdated Show resolved Hide resolved

Lightning-AI deleted a comment from gitguardian bot Aug 15, 2024

apaz-cli and others added 10 commits August 15, 2024 05:58

WIP

e9c4616

removed hf token

Update tests/test_chat.py

c103b16

Co-authored-by: Sebastian Raschka <mail@sebastianraschka.com>

Update tests/test_chat.py

ad2cd48

Co-authored-by: Sebastian Raschka <mail@sebastianraschka.com>

Progress.

906113c

Cleanup.

328d81c

Fixed tests.

9b33ad8

Test Cleanup

0752df9

More Test Cleanup

63a3a47

Stub out test for removed function.

2b80cd3

Remove extra import.

6bd381a

apaz-cli force-pushed the ap/combine_generage branch from af90a4b to 6bd381a Compare August 15, 2024 06:04

apaz-cli marked this pull request as ready for review August 15, 2024 13:53

apaz-cli requested review from awaelchli and lantiga as code owners August 15, 2024 13:53

rasbt reviewed Aug 15, 2024

View reviewed changes

Merge branch 'main' into ap/combine_generage

7a62774

rasbt marked this pull request as draft August 15, 2024 21:57

apaz-cli and others added 9 commits August 16, 2024 18:28

Update comments and fix tests

0a016b1

Merge branch 'ap/combine_generage' of https://github.com/lightning-ai…

28454b3

…/litgpt into ap/combine_generage

Merge branch 'main' into ap/combine_generage

a4fc1c8

Cleaned up tests.

4f3048e

Merge branch 'ap/combine_generage' of https://github.com/lightning-ai…

f5c0094

…/litgpt into ap/combine_generage

fix kv cache bug

ffc8228

Wrote test_decode, fixed subtle type hints.

35115a5

Merge branch 'ap/combine_generage' of https://github.com/lightning-ai…

2d09f22

…/litgpt into ap/combine_generage

Revert "Merge branch 'main' into ap/combine_generage"

5ad8416

This reverts commit a4fc1c8, reversing changes made to 28454b3.

rasbt mentioned this pull request Aug 21, 2024

Make number of generated tokens consistent with CLI #1690

Merged

rasbt reviewed Aug 21, 2024

View reviewed changes

litgpt/generate/base.py Outdated Show resolved Hide resolved

apaz-cli and others added 2 commits August 21, 2024 13:08

Update litgpt/generate/base.py

60c9619

Co-authored-by: Sebastian Raschka <mail@sebastianraschka.com>

Merge branch 'main' into ap/combine_generage

0648421

rasbt reviewed Aug 22, 2024

View reviewed changes

tests/test_api.py Outdated Show resolved Hide resolved

Update tests/test_api.py

5c8223a

rasbt reviewed Aug 22, 2024

View reviewed changes

litgpt/api.py Outdated Show resolved Hide resolved

rasbt reviewed Aug 22, 2024

View reviewed changes

litgpt/api.py Outdated Show resolved Hide resolved

rasbt reviewed Aug 22, 2024

View reviewed changes

litgpt/api.py Outdated Show resolved Hide resolved

rasbt reviewed Aug 22, 2024

View reviewed changes

litgpt/api.py Outdated Show resolved Hide resolved

rasbt reviewed Aug 22, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

rasbt reviewed Aug 22, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

rasbt reviewed Aug 22, 2024

View reviewed changes

apaz-cli and others added 10 commits August 22, 2024 12:17

Update README.md

461be3f

Co-authored-by: Sebastian Raschka <mail@sebastianraschka.com>

Update README.md

b7f7a63

Co-authored-by: Sebastian Raschka <mail@sebastianraschka.com>

Update litgpt/api.py

f46c8dc

Co-authored-by: Sebastian Raschka <mail@sebastianraschka.com>

Update litgpt/api.py

264be3b

Co-authored-by: Sebastian Raschka <mail@sebastianraschka.com>

Update litgpt/api.py

e935d13

Co-authored-by: Sebastian Raschka <mail@sebastianraschka.com>

Cleaned up.

4358fe4

Remove accidentally pushed file.

fc87603

Remove accidentally pushed file.

43f73a4

Cleanup.

362db0b

Fixed input_pos dtype not to depend on torch default dtype.

b9c633e

rasbt reviewed Aug 22, 2024

View reviewed changes

litgpt/generate/base.py Show resolved Hide resolved

rasbt reviewed Aug 22, 2024

View reviewed changes

litgpt/generate/base.py Outdated Show resolved Hide resolved

Update litgpt/generate/base.py

f7f4848

rasbt approved these changes Aug 22, 2024

View reviewed changes

rasbt merged commit 59250d3 into main Aug 22, 2024
8 of 9 checks passed

rasbt deleted the ap/combine_generage branch August 22, 2024 19:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Combine `generate()` functions #1675

Combine `generate()` functions #1675

apaz-cli commented Aug 14, 2024 •

edited

Loading

apaz-cli commented Aug 15, 2024

apaz-cli commented Aug 15, 2024

rasbt left a comment

rasbt Aug 15, 2024

Andrei-Aksionov Aug 16, 2024

rasbt commented Aug 21, 2024

rasbt Aug 22, 2024

apaz-cli Aug 22, 2024

Combine generate() functions #1675

Combine generate() functions #1675

Conversation

apaz-cli commented Aug 14, 2024 • edited Loading

apaz-cli commented Aug 15, 2024

apaz-cli commented Aug 15, 2024

rasbt left a comment

Choose a reason for hiding this comment

rasbt Aug 15, 2024

Choose a reason for hiding this comment

Andrei-Aksionov Aug 16, 2024

Choose a reason for hiding this comment

rasbt commented Aug 21, 2024

rasbt Aug 22, 2024

Choose a reason for hiding this comment

apaz-cli Aug 22, 2024

Choose a reason for hiding this comment

Combine `generate()` functions #1675

Combine `generate()` functions #1675

apaz-cli commented Aug 14, 2024 •

edited

Loading